devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/9] Implement hardware automatic clock gating (HWACG) for gs101
@ 2025-10-13 20:51 Peter Griffin
  2025-10-13 20:51 ` [PATCH 1/9] dt-bindings: soc: samsung: exynos-sysreg: add gs101 hsi0 and misc compatibles Peter Griffin
                   ` (8 more replies)
  0 siblings, 9 replies; 30+ messages in thread
From: Peter Griffin @ 2025-10-13 20:51 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar,
	André Draszik, Tudor Ambarus, Michael Turquette,
	Stephen Boyd, Sam Protsenko, Sylwester Nawrocki, Chanwoo Choi
  Cc: Will McVicker, Krzysztof Kozlowski, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-clk, Krzysztof Kozlowski,
	kernel-team, Peter Griffin

Hi folks,

This series addresses an issue with Samsung Exynos based upstream clock driver
whereby the upstream clock driver sets all the clock gates into "manual mode"
(which uses a bit that is documented as reserved in the gate registers).

Another issue with the current "manual clock gating" approach upstream is
there are many bus/interconnect clocks whose relationships to the IPs
are not well documented or defined in the specs. When adding a new CMU until
now we have tried to label these clocks appropriately with CLK_IS_CRITICAL and
CLK_IGNORE_UNUSED but doing so is both error prone and time consuming. If
your lucky disabling a critical bus clock causes an immediate hang. Other
clocks however aren't so obvious and show up through random instability
some period of time later.

Fortunately each CMU (at least on newer Exynos) provides a "hardware
automatic clock gating" HWACG feature that is used by the downstream
Samsung clock drivers. Hardware automatic clock gating uses a hardware
interface between the CMU and IP to control all clocks required by the
IP. This interface is called Q-channel, and is part of the Arm AMBA low
power interface specification [1].

The advantage of using this Qchannel hardware interface for
enabling/disabling the clocks is that it takes care of all clocks
(including bus/interconnect) ones for the IP automatically thereby reducing
the dynamic power.

Whilst each clock component (GATE, MUX, DIV, QCH etc) has a HWACG enable
bit there are also some "global enable override" bits for the entire CMU in
the CMU_CONTROLLER_OPTION register.

This series makes use of those "global enable" override bits to enable auto
clock mode for the entire CMU and every component within it. Through
experimentation we can see that setting the "manual mode" reserved gate bit
on a particular gate register overides the global enable bits. So the code
is updated accordingly not to do that.

Auto clock mode has been implemented as a "opt in" by setting a new
auto_clock_gate flag in the CMU static data. The intention is existing
platforms in manual mode should not be effected by any of these changes.

If auto_clock_mode flag is set and the option_offset field is specified
then the global enable override bits will be written for the
CMU (to avoid relying on any prior bootstage configuration). Again if auto
mode is enabled the code no longer sets MANUAL and clears HWACG bits on
each gate register.

To have dynamic root clock gating (drcg) of bus components and memclk
enabled, it is required to set the bus_component_drcg and memclk registers
in the the correspondingly named sysreg controller. If auto clock mode is
enabled the clock driver will now attempt to get the sysreg syscon via the
samsung,sysreg property (as used by other Exynos drivers upstream) and set
the registers accordingly. The suspend/resume code paths are also updated
to handle saving/restoring registers using a regmap. Note cmu_top is an
exception and does not have a corresondingly named sysreg_top.

As all clock gates are currently exposed in the gs101 drivers and DT, we
continue to register all of these gates in auto clock mode, but with some new
samsung_auto_clk_gate_ops. As clk enable and clk disable are now handled by
Q-channel interface the .enable and .disable implementations are
no-ops. However by using some CMU qchannel debug registers we can report
the current clock status (enabled or disabled) of every clock gate in the
system. This has the nice effect of still being able to dump the entire
clock tree from /sys/kernel/debug/clk/clk_summary and see a live view of
every auto clock in the system.

With the infrastructure in place, all the CMUs registerred in clk-gs101 are
now updated to enable auto clock mode. From dumping
/sys/kernel/debug/clk/clk_summary it is possible to see that after enabling
auto clock mode approximately 305 clocks are enabled, and 299 are now
disabled. This number goes up and down a bit by 3-5 clocks just on a idle
system sat at a console. As the CLK_IGNORE_UNUSED and CLK_IS_CRITICAL flags
lose any meaning in auto clock mode they are removed. It is now also
possible to boot without the clk_ignore_unused kernel command line property
for the first time! Prior to enabling auto clock mode 586 clocks were
enabled, and 17 disabled (in part due to having to boot with
clk_ignore_unused flag). To ensure compatability with older DTs the
resource size is now checked and an error issued if the DT needs updating to be
compatible with auto clock mode.

For future CMUs in gs101 I propose we continue to expose all gates, but
register the CMU in "auto mode". For new device drivers or updates to
existing dt bindings related to clocks to support gs101 I suggest we only
use the "obviously correct" clock(s). By "obviously correct" I mean a clock
has the IP name in the clock register name, but not try to deduce other
obsucurely named bus/interconnect clocks which will now all be handled
automatically. Note it is still possible to test whether the "obviously
correct" clock is indeed correct by putting the individual gate in manual
mode and disabling the clock (e.g. by using devmem). 

Note: As everything here will go via one of Krzysztof's trees I've sent it
as one series.

regards,

Peter

[1] https://documentation-service.arm.com/static/5f915e69f86e16515cdc3b3e?token=

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
Peter Griffin (9):
      dt-bindings: soc: samsung: exynos-sysreg: add gs101 hsi0 and misc compatibles
      dt-bindings: clock: google,gs101-clock: add samsung,sysreg property as required
      arm64: dts: exynos: gs101: add sysreg_misc and sysreg_hsi0 nodes
      arm64: dts: exynos: gs101: fix clock module unit reg sizes
      arm64: dts: exynos: gs101: fix sysreg_apm reg property
      arm64: dts: exynos: gs101: add samsung,sysreg property to CMU nodes
      clk: samsung: Implement automatic clock gating mode for CMUs
      clk: samsung: gs101: Enable auto_clock_gate mode for each gs101 CMU
      clk: samsung: gs101: remove CLK_IGNORE_UNUSED and CLK_IS_CRITICAL flags

 .../bindings/clock/google,gs101-clock.yaml         |  23 ++-
 .../soc/samsung/samsung,exynos-sysreg.yaml         |   4 +
 arch/arm64/boot/dts/exynos/google/gs101.dtsi       |  36 +++--
 drivers/clk/samsung/clk-exynos-arm64.c             |  47 +++++-
 drivers/clk/samsung/clk-exynos4.c                  |   6 +-
 drivers/clk/samsung/clk-exynos4412-isp.c           |   4 +-
 drivers/clk/samsung/clk-exynos5250.c               |   2 +-
 drivers/clk/samsung/clk-exynos5420.c               |   4 +-
 drivers/clk/samsung/clk-gs101.c                    | 167 ++++++++++++++-------
 drivers/clk/samsung/clk.c                          | 161 ++++++++++++++++++--
 drivers/clk/samsung/clk.h                          |  49 +++++-
 11 files changed, 412 insertions(+), 91 deletions(-)
---
base-commit: 4a71531471926e3c391665ee9c42f4e0295a4585
change-id: 20251008-automatic-clocks-249ab60f62ce

Best regards,
-- 
Peter Griffin <peter.griffin@linaro.org>


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

* [PATCH 1/9] dt-bindings: soc: samsung: exynos-sysreg: add gs101 hsi0 and misc compatibles
  2025-10-13 20:51 [PATCH 0/9] Implement hardware automatic clock gating (HWACG) for gs101 Peter Griffin
@ 2025-10-13 20:51 ` Peter Griffin
  2025-10-15 18:31   ` Rob Herring (Arm)
                     ` (2 more replies)
  2025-10-13 20:51 ` [PATCH 2/9] dt-bindings: clock: google,gs101-clock: add samsung,sysreg property as required Peter Griffin
                   ` (7 subsequent siblings)
  8 siblings, 3 replies; 30+ messages in thread
From: Peter Griffin @ 2025-10-13 20:51 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar,
	André Draszik, Tudor Ambarus, Michael Turquette,
	Stephen Boyd, Sam Protsenko, Sylwester Nawrocki, Chanwoo Choi
  Cc: Will McVicker, Krzysztof Kozlowski, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-clk, Krzysztof Kozlowski,
	kernel-team, Peter Griffin

Add dedicated compatibles for gs101 hsi0 and misc sysreg controllers to the
documentation.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 .../devicetree/bindings/soc/samsung/samsung,exynos-sysreg.yaml        | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-sysreg.yaml b/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-sysreg.yaml
index d8b302f975474a87e4886006cf0b21cf758e4479..289406fb586e1a8a9eccb8eb781f159fd5b9d6eb 100644
--- a/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-sysreg.yaml
+++ b/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-sysreg.yaml
@@ -15,7 +15,9 @@ properties:
       - items:
           - enum:
               - google,gs101-apm-sysreg
+              - google,gs101-hsi0-sysreg
               - google,gs101-hsi2-sysreg
+              - google,gs101-misc-sysreg
               - google,gs101-peric0-sysreg
               - google,gs101-peric1-sysreg
               - samsung,exynos2200-cmgp-sysreg
@@ -83,7 +85,9 @@ allOf:
         compatible:
           contains:
             enum:
+              - google,gs101-hsi0-sysreg
               - google,gs101-hsi2-sysreg
+              - google,gs101-misc-sysreg
               - google,gs101-peric0-sysreg
               - google,gs101-peric1-sysreg
               - samsung,exynos850-cmgp-sysreg

-- 
2.51.0.760.g7b8bcc2412-goog


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

* [PATCH 2/9] dt-bindings: clock: google,gs101-clock: add samsung,sysreg property as required
  2025-10-13 20:51 [PATCH 0/9] Implement hardware automatic clock gating (HWACG) for gs101 Peter Griffin
  2025-10-13 20:51 ` [PATCH 1/9] dt-bindings: soc: samsung: exynos-sysreg: add gs101 hsi0 and misc compatibles Peter Griffin
@ 2025-10-13 20:51 ` Peter Griffin
  2025-10-15 18:32   ` Rob Herring (Arm)
                     ` (2 more replies)
  2025-10-13 20:51 ` [PATCH 3/9] arm64: dts: exynos: gs101: add sysreg_misc and sysreg_hsi0 nodes Peter Griffin
                   ` (6 subsequent siblings)
  8 siblings, 3 replies; 30+ messages in thread
From: Peter Griffin @ 2025-10-13 20:51 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar,
	André Draszik, Tudor Ambarus, Michael Turquette,
	Stephen Boyd, Sam Protsenko, Sylwester Nawrocki, Chanwoo Choi
  Cc: Will McVicker, Krzysztof Kozlowski, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-clk, Krzysztof Kozlowski,
	kernel-team, Peter Griffin

Update the bindings documentation so that all CMUs (with the exception of
gs101-cmu-top) have samsung,sysreg as a required property.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 .../bindings/clock/google,gs101-clock.yaml         | 23 +++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml b/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml
index caf442ead24bda57e531420d8a7d8de8713032ae..5cfe98d9ba895d5207fffc82f3fd55b602b4a2bb 100644
--- a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml
+++ b/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml
@@ -49,6 +49,11 @@ properties:
   reg:
     maxItems: 1
 
+  samsung,sysreg:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Phandle to system registers interface.
+
 required:
   - compatible
   - "#clock-cells"
@@ -163,6 +168,22 @@ allOf:
             - const: bus
             - const: ip
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - google,gs101-cmu-apm
+              - google,gs101-cmu-misc
+              - google,gs101-hsi0
+              - google,gs101-cmu-hsi2
+              - google,gs101-cmu-peric0
+              - google,gs101-cmu-peric1
+
+    then:
+      required:
+        - samsung,sysreg
+
 additionalProperties: false
 
 examples:
@@ -172,7 +193,7 @@ examples:
 
     cmu_top: clock-controller@1e080000 {
         compatible = "google,gs101-cmu-top";
-        reg = <0x1e080000 0x8000>;
+        reg = <0x1e080000 0x10000>;
         #clock-cells = <1>;
         clocks = <&ext_24_5m>;
         clock-names = "oscclk";

-- 
2.51.0.760.g7b8bcc2412-goog


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

* [PATCH 3/9] arm64: dts: exynos: gs101: add sysreg_misc and sysreg_hsi0 nodes
  2025-10-13 20:51 [PATCH 0/9] Implement hardware automatic clock gating (HWACG) for gs101 Peter Griffin
  2025-10-13 20:51 ` [PATCH 1/9] dt-bindings: soc: samsung: exynos-sysreg: add gs101 hsi0 and misc compatibles Peter Griffin
  2025-10-13 20:51 ` [PATCH 2/9] dt-bindings: clock: google,gs101-clock: add samsung,sysreg property as required Peter Griffin
@ 2025-10-13 20:51 ` Peter Griffin
  2025-10-16  5:56   ` André Draszik
  2025-10-21 19:27   ` (subset) " Krzysztof Kozlowski
  2025-10-13 20:51 ` [PATCH 4/9] arm64: dts: exynos: gs101: fix clock module unit reg sizes Peter Griffin
                   ` (5 subsequent siblings)
  8 siblings, 2 replies; 30+ messages in thread
From: Peter Griffin @ 2025-10-13 20:51 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar,
	André Draszik, Tudor Ambarus, Michael Turquette,
	Stephen Boyd, Sam Protsenko, Sylwester Nawrocki, Chanwoo Choi
  Cc: Will McVicker, Krzysztof Kozlowski, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-clk, Krzysztof Kozlowski,
	kernel-team, Peter Griffin

Add syscon DT node for the hsi0 and misc sysreg controllers. These will be
referenced by their respective CMU nodes in future patchs.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 arch/arm64/boot/dts/exynos/google/gs101.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
index 31c99526470d0bb946d498f7546e70c84ed4845b..d1e3226da6472bb9db766926100a6b9855d7a30c 100644
--- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
+++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
@@ -295,6 +295,12 @@ cmu_misc: clock-controller@10010000 {
 			clock-names = "bus", "sss";
 		};
 
+		sysreg_misc: syscon@10030000 {
+			compatible = "google,gs101-misc-sysreg", "syscon";
+			reg = <0x10030000 0x10000>;
+			clocks = <&cmu_misc CLK_GOUT_MISC_SYSREG_MISC_PCLK>;
+		};
+
 		timer@10050000 {
 			compatible = "google,gs101-mct",
 				     "samsung,exynos4210-mct";
@@ -1277,6 +1283,12 @@ cmu_hsi0: clock-controller@11000000 {
 				      "usbdpdbg";
 		};
 
+		sysreg_hsi0: syscon@11020000 {
+			compatible = "google,gs101-hsi0-sysreg", "syscon";
+			reg = <0x11020000 0x10000>;
+			clocks = <&cmu_hsi0 CLK_GOUT_HSI0_SYSREG_HSI0_PCLK>;
+		};
+
 		usbdrd31_phy: phy@11100000 {
 			compatible = "google,gs101-usb31drd-phy";
 			reg = <0x11100000 0x0200>,

-- 
2.51.0.760.g7b8bcc2412-goog


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

* [PATCH 4/9] arm64: dts: exynos: gs101: fix clock module unit reg sizes
  2025-10-13 20:51 [PATCH 0/9] Implement hardware automatic clock gating (HWACG) for gs101 Peter Griffin
                   ` (2 preceding siblings ...)
  2025-10-13 20:51 ` [PATCH 3/9] arm64: dts: exynos: gs101: add sysreg_misc and sysreg_hsi0 nodes Peter Griffin
@ 2025-10-13 20:51 ` Peter Griffin
  2025-10-16  8:22   ` André Draszik
  2025-10-21 19:27   ` (subset) " Krzysztof Kozlowski
  2025-10-13 20:51 ` [PATCH 5/9] arm64: dts: exynos: gs101: fix sysreg_apm reg property Peter Griffin
                   ` (4 subsequent siblings)
  8 siblings, 2 replies; 30+ messages in thread
From: Peter Griffin @ 2025-10-13 20:51 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar,
	André Draszik, Tudor Ambarus, Michael Turquette,
	Stephen Boyd, Sam Protsenko, Sylwester Nawrocki, Chanwoo Choi
  Cc: Will McVicker, Krzysztof Kozlowski, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-clk, Krzysztof Kozlowski,
	kernel-team, Peter Griffin

The memory map lists each clock module unit as having a size of
0x10000. Additionally there are some undocumented registers in this region
that need to be used for automatic clock gating mode. Some of those
registers also need to be saved/restored on suspend & resume.

Fixes: 86124c76683e ("arm64: dts: exynos: gs101: enable cmu-hsi2 clock controller")
Fixes: 4982a4a2092e ("arm64: dts: exynos: gs101: enable cmu-hsi0 clock controller")
Fixes: 7d66d98b5bf3 ("arm64: dts: exynos: gs101: enable cmu-peric1 clock controller")
Fixes: e62c706f3aa0 ("arm64: dts: exynos: gs101: enable cmu-peric0 clock controller")
Fixes: ea89fdf24fd9 ("arm64: dts: exynos: google: Add initial Google gs101 SoC support")
Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 arch/arm64/boot/dts/exynos/google/gs101.dtsi | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
index d1e3226da6472bb9db766926100a6b9855d7a30c..1ae965e456665bf05aa1b08269b5dd66b46d200b 100644
--- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
+++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
@@ -288,7 +288,7 @@ soc: soc@0 {
 
 		cmu_misc: clock-controller@10010000 {
 			compatible = "google,gs101-cmu-misc";
-			reg = <0x10010000 0x8000>;
+			reg = <0x10010000 0x10000>;
 			#clock-cells = <1>;
 			clocks = <&cmu_top CLK_DOUT_CMU_MISC_BUS>,
 				 <&cmu_top CLK_DOUT_CMU_MISC_SSS>;
@@ -371,7 +371,7 @@ ppi_cluster2: interrupt-partition-2 {
 
 		cmu_peric0: clock-controller@10800000 {
 			compatible = "google,gs101-cmu-peric0";
-			reg = <0x10800000 0x4000>;
+			reg = <0x10800000 0x10000>;
 			#clock-cells = <1>;
 			clocks = <&ext_24_5m>,
 				 <&cmu_top CLK_DOUT_CMU_PERIC0_BUS>,
@@ -917,7 +917,7 @@ spi_14: spi@10a20000 {
 
 		cmu_peric1: clock-controller@10c00000 {
 			compatible = "google,gs101-cmu-peric1";
-			reg = <0x10c00000 0x4000>;
+			reg = <0x10c00000 0x10000>;
 			#clock-cells = <1>;
 			clocks = <&ext_24_5m>,
 				 <&cmu_top CLK_DOUT_CMU_PERIC1_BUS>,
@@ -1271,7 +1271,7 @@ spi_13: spi@10d60000 {
 
 		cmu_hsi0: clock-controller@11000000 {
 			compatible = "google,gs101-cmu-hsi0";
-			reg = <0x11000000 0x4000>;
+			reg = <0x11000000 0x10000>;
 			#clock-cells = <1>;
 
 			clocks = <&ext_24_5m>,
@@ -1344,7 +1344,7 @@ pinctrl_hsi1: pinctrl@11840000 {
 
 		cmu_hsi2: clock-controller@14400000 {
 			compatible = "google,gs101-cmu-hsi2";
-			reg = <0x14400000 0x4000>;
+			reg = <0x14400000 0x10000>;
 			#clock-cells = <1>;
 			clocks = <&ext_24_5m>,
 				 <&cmu_top CLK_DOUT_CMU_HSI2_BUS>,
@@ -1407,7 +1407,7 @@ ufs_0_phy: phy@14704000 {
 
 		cmu_apm: clock-controller@17400000 {
 			compatible = "google,gs101-cmu-apm";
-			reg = <0x17400000 0x8000>;
+			reg = <0x17400000 0x10000>;
 			#clock-cells = <1>;
 
 			clocks = <&ext_24_5m>;
@@ -1509,7 +1509,7 @@ pinctrl_gsacore: pinctrl@17a80000 {
 
 		cmu_top: clock-controller@1e080000 {
 			compatible = "google,gs101-cmu-top";
-			reg = <0x1e080000 0x8000>;
+			reg = <0x1e080000 0x10000>;
 			#clock-cells = <1>;
 
 			clocks = <&ext_24_5m>;

-- 
2.51.0.760.g7b8bcc2412-goog


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

* [PATCH 5/9] arm64: dts: exynos: gs101: fix sysreg_apm reg property
  2025-10-13 20:51 [PATCH 0/9] Implement hardware automatic clock gating (HWACG) for gs101 Peter Griffin
                   ` (3 preceding siblings ...)
  2025-10-13 20:51 ` [PATCH 4/9] arm64: dts: exynos: gs101: fix clock module unit reg sizes Peter Griffin
@ 2025-10-13 20:51 ` Peter Griffin
  2025-10-16  8:23   ` André Draszik
  2025-10-21 19:27   ` (subset) " Krzysztof Kozlowski
  2025-10-13 20:51 ` [PATCH 6/9] arm64: dts: exynos: gs101: add samsung,sysreg property to CMU nodes Peter Griffin
                   ` (3 subsequent siblings)
  8 siblings, 2 replies; 30+ messages in thread
From: Peter Griffin @ 2025-10-13 20:51 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar,
	André Draszik, Tudor Ambarus, Michael Turquette,
	Stephen Boyd, Sam Protsenko, Sylwester Nawrocki, Chanwoo Choi
  Cc: Will McVicker, Krzysztof Kozlowski, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-clk, Krzysztof Kozlowski,
	kernel-team, Peter Griffin

Both the start address and size are incorrect for the apm_sysreg DT
node. Update to match the TRM (rather than how it was defined downstream).

Fixes: ea89fdf24fd9 ("arm64: dts: exynos: google: Add initial Google gs101 SoC support")
Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 arch/arm64/boot/dts/exynos/google/gs101.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
index 1ae965e456665bf05aa1b08269b5dd66b46d200b..ab66c055e0ac157f89a0e034f15bbe84e20a7e82 100644
--- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
+++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
@@ -1414,9 +1414,9 @@ cmu_apm: clock-controller@17400000 {
 			clock-names = "oscclk";
 		};
 
-		sysreg_apm: syscon@174204e0 {
+		sysreg_apm: syscon@17420000 {
 			compatible = "google,gs101-apm-sysreg", "syscon";
-			reg = <0x174204e0 0x1000>;
+			reg = <0x17420000 0x10000>;
 		};
 
 		pmu_system_controller: system-controller@17460000 {

-- 
2.51.0.760.g7b8bcc2412-goog


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

* [PATCH 6/9] arm64: dts: exynos: gs101: add samsung,sysreg property to CMU nodes
  2025-10-13 20:51 [PATCH 0/9] Implement hardware automatic clock gating (HWACG) for gs101 Peter Griffin
                   ` (4 preceding siblings ...)
  2025-10-13 20:51 ` [PATCH 5/9] arm64: dts: exynos: gs101: fix sysreg_apm reg property Peter Griffin
@ 2025-10-13 20:51 ` Peter Griffin
  2025-10-13 20:51 ` [PATCH 7/9] clk: samsung: Implement automatic clock gating mode for CMUs Peter Griffin
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 30+ messages in thread
From: Peter Griffin @ 2025-10-13 20:51 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar,
	André Draszik, Tudor Ambarus, Michael Turquette,
	Stephen Boyd, Sam Protsenko, Sylwester Nawrocki, Chanwoo Choi
  Cc: Will McVicker, Krzysztof Kozlowski, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-clk, Krzysztof Kozlowski,
	kernel-team, Peter Griffin

Each CMU (with the exception of cmu_top) should have a phandle to its
simarlarly named sysreg controller.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 arch/arm64/boot/dts/exynos/google/gs101.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
index ab66c055e0ac157f89a0e034f15bbe84e20a7e82..c54468ddbb02b170ec79d56ba2460f2ffb0dc40d 100644
--- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
+++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
@@ -293,6 +293,7 @@ cmu_misc: clock-controller@10010000 {
 			clocks = <&cmu_top CLK_DOUT_CMU_MISC_BUS>,
 				 <&cmu_top CLK_DOUT_CMU_MISC_SSS>;
 			clock-names = "bus", "sss";
+			samsung,sysreg = <&sysreg_misc>;
 		};
 
 		sysreg_misc: syscon@10030000 {
@@ -377,6 +378,7 @@ cmu_peric0: clock-controller@10800000 {
 				 <&cmu_top CLK_DOUT_CMU_PERIC0_BUS>,
 				 <&cmu_top CLK_DOUT_CMU_PERIC0_IP>;
 			clock-names = "oscclk", "bus", "ip";
+			samsung,sysreg = <&sysreg_peric0>;
 		};
 
 		sysreg_peric0: syscon@10820000 {
@@ -923,6 +925,7 @@ cmu_peric1: clock-controller@10c00000 {
 				 <&cmu_top CLK_DOUT_CMU_PERIC1_BUS>,
 				 <&cmu_top CLK_DOUT_CMU_PERIC1_IP>;
 			clock-names = "oscclk", "bus", "ip";
+			samsung,sysreg = <&sysreg_peric1>;
 		};
 
 		sysreg_peric1: syscon@10c20000 {
@@ -1281,6 +1284,7 @@ cmu_hsi0: clock-controller@11000000 {
 				 <&cmu_top CLK_DOUT_CMU_HSI0_USBDPDBG>;
 			clock-names = "oscclk", "bus", "dpgtc", "usb31drd",
 				      "usbdpdbg";
+			samsung,sysreg = <&sysreg_hsi0>;
 		};
 
 		sysreg_hsi0: syscon@11020000 {
@@ -1352,6 +1356,7 @@ cmu_hsi2: clock-controller@14400000 {
 				 <&cmu_top CLK_DOUT_CMU_HSI2_UFS_EMBD>,
 				 <&cmu_top CLK_DOUT_CMU_HSI2_MMC_CARD>;
 			clock-names = "oscclk", "bus", "pcie", "ufs", "mmc";
+			samsung,sysreg = <&sysreg_hsi2>;
 		};
 
 		sysreg_hsi2: syscon@14420000 {
@@ -1412,6 +1417,7 @@ cmu_apm: clock-controller@17400000 {
 
 			clocks = <&ext_24_5m>;
 			clock-names = "oscclk";
+			samsung,sysreg = <&sysreg_apm>;
 		};
 
 		sysreg_apm: syscon@17420000 {

-- 
2.51.0.760.g7b8bcc2412-goog


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

* [PATCH 7/9] clk: samsung: Implement automatic clock gating mode for CMUs
  2025-10-13 20:51 [PATCH 0/9] Implement hardware automatic clock gating (HWACG) for gs101 Peter Griffin
                   ` (5 preceding siblings ...)
  2025-10-13 20:51 ` [PATCH 6/9] arm64: dts: exynos: gs101: add samsung,sysreg property to CMU nodes Peter Griffin
@ 2025-10-13 20:51 ` Peter Griffin
  2025-10-14 14:46   ` kernel test robot
                     ` (2 more replies)
  2025-10-13 20:51 ` [PATCH 8/9] clk: samsung: gs101: Enable auto_clock_gate mode for each gs101 CMU Peter Griffin
  2025-10-13 20:51 ` [PATCH 9/9] clk: samsung: gs101: remove CLK_IGNORE_UNUSED and CLK_IS_CRITICAL flags Peter Griffin
  8 siblings, 3 replies; 30+ messages in thread
From: Peter Griffin @ 2025-10-13 20:51 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar,
	André Draszik, Tudor Ambarus, Michael Turquette,
	Stephen Boyd, Sam Protsenko, Sylwester Nawrocki, Chanwoo Choi
  Cc: Will McVicker, Krzysztof Kozlowski, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-clk, Krzysztof Kozlowski,
	kernel-team, Peter Griffin

Update exynos_arm64_init_clocks() so that it enables the automatic clock
mode bits in the CMU option register if the auto_clock_gate flag and
option_offset fields are set for the CMU.

The CMU option register bits are global and effect every clock component in
the CMU, as such clearing the GATE_ENABLE_HWACG bit and setting GATE_MANUAL
bit on every gate register is only required if auto_clock_gate is false.

Additionally if auto_clock_gate is enabled the dynamic root clock gating
and memclk registers will be configured in the corresponding CMUs sysreg
bank. These registers are exposed via syscon, so the register
suspend/resume paths are updated to handle using a regmap.

As many gates for various Samsung SoCs are already exposed in the Samsung
clock drivers a new samsung_auto_clk_gate_ops is implemented. This uses
some CMU debug registers to report whether clocks are enabled or disabled
when operating in automatic mode. This allows
/sys/kernel/debug/clk/clk_summary to still dump the entire clock tree and
correctly report the status of each clock in the system.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 drivers/clk/samsung/clk-exynos-arm64.c   |  47 +++++++--
 drivers/clk/samsung/clk-exynos4.c        |   6 +-
 drivers/clk/samsung/clk-exynos4412-isp.c |   4 +-
 drivers/clk/samsung/clk-exynos5250.c     |   2 +-
 drivers/clk/samsung/clk-exynos5420.c     |   4 +-
 drivers/clk/samsung/clk.c                | 161 ++++++++++++++++++++++++++++---
 drivers/clk/samsung/clk.h                |  49 +++++++++-
 7 files changed, 244 insertions(+), 29 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos-arm64.c b/drivers/clk/samsung/clk-exynos-arm64.c
index bf7de21f329ec89069dcf817ca578fcf9b2d9809..c302c836e8f9f6270753d86d7d986c88e6762f4f 100644
--- a/drivers/clk/samsung/clk-exynos-arm64.c
+++ b/drivers/clk/samsung/clk-exynos-arm64.c
@@ -24,6 +24,16 @@
 #define GATE_MANUAL		BIT(20)
 #define GATE_ENABLE_HWACG	BIT(28)
 
+/* Option register bits */
+#define OPT_EN_DBG			BIT(31)
+#define OPT_UNKNOWN			BIT(30)
+#define OPT_EN_PWR_MANAGEMENT		BIT(29)
+#define OPT_EN_AUTO_GATING		BIT(28)
+#define OPT_EN_MEM_PM_GATING		BIT(24)
+
+#define CMU_OPT_GLOBAL_EN_AUTO_GATING	(OPT_EN_DBG | OPT_UNKNOWN | \
+	OPT_EN_PWR_MANAGEMENT | OPT_EN_AUTO_GATING | OPT_EN_MEM_PM_GATING)
+
 /* PLL_CONx_PLL register offsets range */
 #define PLL_CON_OFF_START	0x100
 #define PLL_CON_OFF_END		0x600
@@ -37,6 +47,8 @@ struct exynos_arm64_cmu_data {
 	unsigned int nr_clk_save;
 	const struct samsung_clk_reg_dump *clk_suspend;
 	unsigned int nr_clk_suspend;
+	struct samsung_clk_reg_dump *clk_sysreg_save;
+	unsigned int nr_clk_sysreg;
 
 	struct clk *clk;
 	struct clk **pclks;
@@ -82,13 +94,28 @@ static void __init exynos_arm64_init_clocks(struct device_node *np,
 	if (!reg_base)
 		panic("%s: failed to map registers\n", __func__);
 
+	if (cmu->auto_clock_gate && cmu->option_offset) {
+		/*
+		 * Enable the global automatic mode for the entire CMU.
+		 * This overrides the individual HWACG bits in each of the
+		 * individual gate, mux and qch registers.
+		 */
+		writel(CMU_OPT_GLOBAL_EN_AUTO_GATING,
+		       reg_base + cmu->option_offset);
+	}
+
 	for (i = 0; i < reg_offs_len; ++i) {
 		void __iomem *reg = reg_base + reg_offs[i];
 		u32 val;
 
 		if (cmu->manual_plls && is_pll_con1_reg(reg_offs[i])) {
 			writel(PLL_CON1_MANUAL, reg);
-		} else if (is_gate_reg(reg_offs[i])) {
+		} else if (is_gate_reg(reg_offs[i]) && !cmu->auto_clock_gate) {
+			/*
+			 * Setting GATE_MANUAL bit (which is described in TRM as
+			 * reserved!) overrides the global CMU automatic mode
+			 * option.
+			 */
 			val = readl(reg);
 			val |= GATE_MANUAL;
 			val &= ~GATE_ENABLE_HWACG;
@@ -219,7 +246,7 @@ void __init exynos_arm64_register_cmu(struct device *dev,
  * Return: 0 on success, or negative error code on error.
  */
 int __init exynos_arm64_register_cmu_pm(struct platform_device *pdev,
-					bool set_manual)
+					bool init_clk_regs)
 {
 	const struct samsung_cmu_info *cmu;
 	struct device *dev = &pdev->dev;
@@ -249,7 +276,7 @@ int __init exynos_arm64_register_cmu_pm(struct platform_device *pdev,
 		dev_err(dev, "%s: could not enable bus clock %s; err = %d\n",
 		       __func__, cmu->clk_name, ret);
 
-	if (set_manual)
+	if (init_clk_regs)
 		exynos_arm64_init_clocks(np, cmu);
 
 	reg_base = devm_platform_ioremap_resource(pdev, 0);
@@ -280,14 +307,18 @@ int exynos_arm64_cmu_suspend(struct device *dev)
 	struct exynos_arm64_cmu_data *data = dev_get_drvdata(dev);
 	int i;
 
-	samsung_clk_save(data->ctx->reg_base, data->clk_save,
+	samsung_clk_save(data->ctx->reg_base, NULL, data->clk_save,
 			 data->nr_clk_save);
 
+	if (data->ctx->sysreg)
+		samsung_clk_save(NULL, data->ctx->sysreg, data->clk_save,
+				 data->nr_clk_save);
+
 	for (i = 0; i < data->nr_pclks; i++)
 		clk_prepare_enable(data->pclks[i]);
 
 	/* For suspend some registers have to be set to certain values */
-	samsung_clk_restore(data->ctx->reg_base, data->clk_suspend,
+	samsung_clk_restore(data->ctx->reg_base, NULL, data->clk_suspend,
 			    data->nr_clk_suspend);
 
 	for (i = 0; i < data->nr_pclks; i++)
@@ -308,9 +339,13 @@ int exynos_arm64_cmu_resume(struct device *dev)
 	for (i = 0; i < data->nr_pclks; i++)
 		clk_prepare_enable(data->pclks[i]);
 
-	samsung_clk_restore(data->ctx->reg_base, data->clk_save,
+	samsung_clk_restore(data->ctx->reg_base, NULL, data->clk_save,
 			    data->nr_clk_save);
 
+	if (data->ctx->sysreg)
+		samsung_clk_restore(NULL, data->ctx->sysreg, data->clk_save,
+				    data->nr_clk_save);
+
 	for (i = 0; i < data->nr_pclks; i++)
 		clk_disable_unprepare(data->pclks[i]);
 
diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
index cc5c1644c41c08b27bc48d809a08cd8a006cbe8f..26ac9734722d1e7ed8ec3f1c0a956f26e32b92d4 100644
--- a/drivers/clk/samsung/clk-exynos4.c
+++ b/drivers/clk/samsung/clk-exynos4.c
@@ -1378,15 +1378,15 @@ static void __init exynos4_clk_init(struct device_node *np,
 	if (soc == EXYNOS4212 || soc == EXYNOS4412)
 		exynos4x12_core_down_clock();
 
-	samsung_clk_extended_sleep_init(reg_base,
+	samsung_clk_extended_sleep_init(reg_base, NULL,
 			exynos4_clk_regs, ARRAY_SIZE(exynos4_clk_regs),
 			src_mask_suspend, ARRAY_SIZE(src_mask_suspend));
 	if (exynos4_soc == EXYNOS4210)
-		samsung_clk_extended_sleep_init(reg_base,
+		samsung_clk_extended_sleep_init(reg_base, NULL,
 		    exynos4210_clk_save, ARRAY_SIZE(exynos4210_clk_save),
 		    src_mask_suspend_e4210, ARRAY_SIZE(src_mask_suspend_e4210));
 	else
-		samsung_clk_sleep_init(reg_base, exynos4x12_clk_save,
+		samsung_clk_sleep_init(reg_base, NULL, exynos4x12_clk_save,
 				       ARRAY_SIZE(exynos4x12_clk_save));
 
 	samsung_clk_of_add_provider(np, ctx);
diff --git a/drivers/clk/samsung/clk-exynos4412-isp.c b/drivers/clk/samsung/clk-exynos4412-isp.c
index fa915057e109e0008ebe0b1b5d1652fd5804e82b..772bc18a1e686f23b11bf160b803becff6279637 100644
--- a/drivers/clk/samsung/clk-exynos4412-isp.c
+++ b/drivers/clk/samsung/clk-exynos4412-isp.c
@@ -94,7 +94,7 @@ static int __maybe_unused exynos4x12_isp_clk_suspend(struct device *dev)
 {
 	struct samsung_clk_provider *ctx = dev_get_drvdata(dev);
 
-	samsung_clk_save(ctx->reg_base, exynos4x12_save_isp,
+	samsung_clk_save(ctx->reg_base, NULL, exynos4x12_save_isp,
 			 ARRAY_SIZE(exynos4x12_clk_isp_save));
 	return 0;
 }
@@ -103,7 +103,7 @@ static int __maybe_unused exynos4x12_isp_clk_resume(struct device *dev)
 {
 	struct samsung_clk_provider *ctx = dev_get_drvdata(dev);
 
-	samsung_clk_restore(ctx->reg_base, exynos4x12_save_isp,
+	samsung_clk_restore(ctx->reg_base, NULL, exynos4x12_save_isp,
 			    ARRAY_SIZE(exynos4x12_clk_isp_save));
 	return 0;
 }
diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c
index e90d3a0848cbc24b2709c10795f6affcda404567..f97f30b29be7317db8186bac39cf52e1893eb106 100644
--- a/drivers/clk/samsung/clk-exynos5250.c
+++ b/drivers/clk/samsung/clk-exynos5250.c
@@ -854,7 +854,7 @@ static void __init exynos5250_clk_init(struct device_node *np)
 		PWR_CTRL2_CORE2_UP_RATIO | PWR_CTRL2_CORE1_UP_RATIO);
 	__raw_writel(tmp, reg_base + PWR_CTRL2);
 
-	samsung_clk_sleep_init(reg_base, exynos5250_clk_regs,
+	samsung_clk_sleep_init(reg_base, NULL, exynos5250_clk_regs,
 			       ARRAY_SIZE(exynos5250_clk_regs));
 	exynos5_subcmus_init(ctx, ARRAY_SIZE(exynos5250_subcmus),
 			     exynos5250_subcmus);
diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
index a9df4e6db82fa7831d4e5c7210b0163d7d301ec1..1982e0751ceec7e57f9e82d96dcbadce1f691092 100644
--- a/drivers/clk/samsung/clk-exynos5420.c
+++ b/drivers/clk/samsung/clk-exynos5420.c
@@ -1649,12 +1649,12 @@ static void __init exynos5x_clk_init(struct device_node *np,
 				ARRAY_SIZE(exynos5800_cpu_clks));
 	}
 
-	samsung_clk_extended_sleep_init(reg_base,
+	samsung_clk_extended_sleep_init(reg_base, NULL,
 		exynos5x_clk_regs, ARRAY_SIZE(exynos5x_clk_regs),
 		exynos5420_set_clksrc, ARRAY_SIZE(exynos5420_set_clksrc));
 
 	if (soc == EXYNOS5800) {
-		samsung_clk_sleep_init(reg_base, exynos5800_clk_regs,
+		samsung_clk_sleep_init(reg_base, NULL, exynos5800_clk_regs,
 				       ARRAY_SIZE(exynos5800_clk_regs));
 
 		exynos5_subcmus_init(ctx, ARRAY_SIZE(exynos5800_subcmus),
diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c
index dbc9925ca8f46e951dfb5d391c0e744ca370abcc..07b2948ae7ea48f126ab420be57d8c2705979464 100644
--- a/drivers/clk/samsung/clk.c
+++ b/drivers/clk/samsung/clk.c
@@ -12,8 +12,10 @@
 #include <linux/clkdev.h>
 #include <linux/clk-provider.h>
 #include <linux/io.h>
+#include <linux/mfd/syscon.h>
 #include <linux/mod_devicetable.h>
 #include <linux/of_address.h>
+#include <linux/regmap.h>
 #include <linux/syscore_ops.h>
 
 #include "clk.h"
@@ -21,19 +23,29 @@
 static LIST_HEAD(clock_reg_cache_list);
 
 void samsung_clk_save(void __iomem *base,
+				    struct regmap *regmap,
 				    struct samsung_clk_reg_dump *rd,
 				    unsigned int num_regs)
 {
-	for (; num_regs > 0; --num_regs, ++rd)
-		rd->value = readl(base + rd->offset);
+	for (; num_regs > 0; --num_regs, ++rd) {
+		if (base)
+			rd->value = readl(base + rd->offset);
+		if (regmap)
+			regmap_read(regmap, rd->offset, &rd->value);
+	}
 }
 
 void samsung_clk_restore(void __iomem *base,
+				      struct regmap *regmap,
 				      const struct samsung_clk_reg_dump *rd,
 				      unsigned int num_regs)
 {
-	for (; num_regs > 0; --num_regs, ++rd)
-		writel(rd->value, base + rd->offset);
+	for (; num_regs > 0; --num_regs, ++rd) {
+		if (base)
+			writel(rd->value, base + rd->offset);
+		if (regmap)
+			regmap_write(regmap, rd->offset, rd->value);
+	}
 }
 
 struct samsung_clk_reg_dump *samsung_clk_alloc_reg_dump(
@@ -227,6 +239,82 @@ void __init samsung_clk_register_div(struct samsung_clk_provider *ctx,
 	}
 }
 
+#define ACG_MSK GENMASK(6, 4)
+#define CLK_IDLE GENMASK(5, 4)
+static int samsung_auto_clk_gate_is_en(struct clk_hw *hw)
+{
+	u32 reg;
+	struct clk_gate *gate = to_clk_gate(hw);
+
+	reg = readl(gate->reg);
+	return ((reg & ACG_MSK) == CLK_IDLE) ? 0 : 1;
+}
+
+/* enable and disable are nops in automatic clock mode */
+static int samsung_auto_clk_gate_en(struct clk_hw *hw)
+{
+	return 0;
+}
+
+static void samsung_auto_clk_gate_dis(struct clk_hw *hw)
+{
+}
+
+static const struct clk_ops samsung_auto_clk_gate_ops = {
+	.enable = samsung_auto_clk_gate_en,
+	.disable = samsung_auto_clk_gate_dis,
+	.is_enabled = samsung_auto_clk_gate_is_en,
+};
+
+struct clk_hw *samsung_register_auto_gate(struct device *dev,
+		struct device_node *np, const char *name,
+		const char *parent_name, const struct clk_hw *parent_hw,
+		const struct clk_parent_data *parent_data,
+		unsigned long flags,
+		void __iomem *reg, u8 bit_idx,
+		u8 clk_gate_flags, spinlock_t *lock)
+{
+	struct clk_gate *gate;
+	struct clk_hw *hw;
+	struct clk_init_data init = {};
+	int ret = -EINVAL;
+
+	/* allocate the gate */
+	gate = kzalloc(sizeof(*gate), GFP_KERNEL);
+	if (!gate)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = name;
+	init.ops = &samsung_auto_clk_gate_ops;
+	init.flags = flags;
+	init.parent_names = parent_name ? &parent_name : NULL;
+	init.parent_hws = parent_hw ? &parent_hw : NULL;
+	init.parent_data = parent_data;
+	if (parent_name || parent_hw || parent_data)
+		init.num_parents = 1;
+	else
+		init.num_parents = 0;
+
+	/* struct clk_gate assignments */
+	gate->reg = reg;
+	gate->bit_idx = bit_idx;
+	gate->flags = clk_gate_flags;
+	gate->lock = lock;
+	gate->hw.init = &init;
+
+	hw = &gate->hw;
+	if (dev || !np)
+		ret = clk_hw_register(dev, hw);
+	else if (np)
+		ret = of_clk_hw_register(np, hw);
+	if (ret) {
+		kfree(gate);
+		hw = ERR_PTR(ret);
+	}
+
+	return hw;
+}
+
 /* register a list of gate clocks */
 void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx,
 				const struct samsung_gate_clock *list,
@@ -234,11 +322,21 @@ void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx,
 {
 	struct clk_hw *clk_hw;
 	unsigned int idx;
+	void __iomem *reg_offs;
 
 	for (idx = 0; idx < nr_clk; idx++, list++) {
-		clk_hw = clk_hw_register_gate(ctx->dev, list->name, list->parent_name,
-				list->flags, ctx->reg_base + list->offset,
+		reg_offs = ctx->reg_base + list->offset;
+
+		if (ctx->auto_clock_gate && ctx->gate_dbg_offset)
+			clk_hw = samsung_register_auto_gate(ctx->dev, NULL,
+				list->name, list->parent_name, NULL, NULL,
+				list->flags, reg_offs + ctx->gate_dbg_offset,
 				list->bit_idx, list->gate_flags, &ctx->lock);
+		else
+			clk_hw = clk_hw_register_gate(ctx->dev, list->name,
+				list->parent_name, list->flags,
+				ctx->reg_base + list->offset, list->bit_idx,
+				list->gate_flags, &ctx->lock);
 		if (IS_ERR(clk_hw)) {
 			pr_err("%s: failed to register clock %s\n", __func__,
 				list->name);
@@ -276,10 +374,11 @@ static int samsung_clk_suspend(void)
 	struct samsung_clock_reg_cache *reg_cache;
 
 	list_for_each_entry(reg_cache, &clock_reg_cache_list, node) {
-		samsung_clk_save(reg_cache->reg_base, reg_cache->rdump,
-				reg_cache->rd_num);
-		samsung_clk_restore(reg_cache->reg_base, reg_cache->rsuspend,
-				reg_cache->rsuspend_num);
+		samsung_clk_save(reg_cache->reg_base, reg_cache->sysreg,
+				 reg_cache->rdump, reg_cache->rd_num);
+		samsung_clk_restore(reg_cache->reg_base, reg_cache->sysreg,
+				    reg_cache->rsuspend,
+				    reg_cache->rsuspend_num);
 	}
 	return 0;
 }
@@ -289,8 +388,8 @@ static void samsung_clk_resume(void)
 	struct samsung_clock_reg_cache *reg_cache;
 
 	list_for_each_entry(reg_cache, &clock_reg_cache_list, node)
-		samsung_clk_restore(reg_cache->reg_base, reg_cache->rdump,
-				reg_cache->rd_num);
+		samsung_clk_restore(reg_cache->reg_base, reg_cache->sysreg,
+				    reg_cache->rdump, reg_cache->rd_num);
 }
 
 static struct syscore_ops samsung_clk_syscore_ops = {
@@ -299,6 +398,7 @@ static struct syscore_ops samsung_clk_syscore_ops = {
 };
 
 void samsung_clk_extended_sleep_init(void __iomem *reg_base,
+			struct regmap *sysreg,
 			const unsigned long *rdump,
 			unsigned long nr_rdump,
 			const struct samsung_clk_reg_dump *rsuspend,
@@ -319,6 +419,7 @@ void samsung_clk_extended_sleep_init(void __iomem *reg_base,
 		register_syscore_ops(&samsung_clk_syscore_ops);
 
 	reg_cache->reg_base = reg_base;
+	reg_cache->sysreg = sysreg;
 	reg_cache->rd_num = nr_rdump;
 	reg_cache->rsuspend = rsuspend;
 	reg_cache->rsuspend_num = nr_rsuspend;
@@ -334,6 +435,12 @@ void samsung_clk_extended_sleep_init(void __iomem *reg_base,
 void __init samsung_cmu_register_clocks(struct samsung_clk_provider *ctx,
 					const struct samsung_cmu_info *cmu)
 {
+	ctx->auto_clock_gate = cmu->auto_clock_gate;
+	ctx->gate_dbg_offset = cmu->gate_dbg_offset;
+	ctx->option_offset = cmu->option_offset;
+	ctx->drcg_offset = cmu->drcg_offset;
+	ctx->memclk_offset = cmu->memclk_offset;
+
 	if (cmu->pll_clks)
 		samsung_clk_register_pll(ctx, cmu->pll_clks, cmu->nr_pll_clks);
 	if (cmu->mux_clks)
@@ -353,6 +460,31 @@ void __init samsung_cmu_register_clocks(struct samsung_clk_provider *ctx,
 		samsung_clk_register_cpu(ctx, cmu->cpu_clks, cmu->nr_cpu_clks);
 }
 
+/* Enable Dynamic Root Clock Gating of bus components*/
+void samsung_en_dyn_root_clk_gating(struct device_node *np,
+				    struct samsung_clk_provider *ctx,
+				    const struct samsung_cmu_info *cmu)
+{
+	if (ctx && !ctx->auto_clock_gate)
+		return;
+
+	ctx->sysreg = syscon_regmap_lookup_by_phandle(np, "samsung,sysreg");
+	if (!IS_ERR_OR_NULL(ctx->sysreg)) {
+		regmap_write(ctx->sysreg, ctx->drcg_offset, 0xffffffff);
+		/* not every sysreg controller has memclk reg*/
+		if (ctx->memclk_offset)
+			regmap_write_bits(ctx->sysreg, ctx->memclk_offset, 0x1, 0x0);
+
+		samsung_clk_extended_sleep_init(NULL, ctx->sysreg,
+						cmu->sysreg_clk_regs,
+						cmu->nr_sysreg_clk_regs,
+						NULL, 0);
+	} else {
+		pr_warn("%pOF: Unable to get CMU sysreg\n", np);
+		ctx->sysreg = NULL;
+	}
+}
+
 /*
  * Common function which registers plls, muxes, dividers and gates
  * for each CMU. It also add CMU register list to register cache.
@@ -374,11 +506,14 @@ struct samsung_clk_provider * __init samsung_cmu_register_one(
 	samsung_cmu_register_clocks(ctx, cmu);
 
 	if (cmu->clk_regs)
-		samsung_clk_extended_sleep_init(reg_base,
+		samsung_clk_extended_sleep_init(reg_base, NULL,
 			cmu->clk_regs, cmu->nr_clk_regs,
 			cmu->suspend_regs, cmu->nr_suspend_regs);
 
 	samsung_clk_of_add_provider(np, ctx);
 
+	/* sysreg DT nodes reference a clock in this CMU */
+	samsung_en_dyn_root_clk_gating(np, ctx, cmu);
+
 	return ctx;
 }
diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
index 18660c1ac6f0106b17b9efc9c6b3cd62d46f7b82..b719e057f45489e9d92ba54031fe633a8c9264ce 100644
--- a/drivers/clk/samsung/clk.h
+++ b/drivers/clk/samsung/clk.h
@@ -12,6 +12,7 @@
 
 #include <linux/clk-provider.h>
 #include <linux/mod_devicetable.h>
+#include <linux/regmap.h>
 #include "clk-pll.h"
 #include "clk-cpu.h"
 
@@ -19,13 +20,25 @@
  * struct samsung_clk_provider - information about clock provider
  * @reg_base: virtual address for the register base
  * @dev: clock provider device needed for runtime PM
+ * @sysreg: syscon regmap for clock-provider sysreg controller
  * @lock: maintains exclusion between callbacks for a given clock-provider
+ * @auto_clock_gate: enable auto clk mode for all clocks in clock-provider
+ * @gate_dbg_offset: gate debug reg offset. Used for all gates in auto clk mode
+ * @option_offset: option reg offset. Enables auto mode for clock-provider
+ * @drcg_offset: dynamic root clk gate enable register offset
+ * @memclk_offset: memclk enable register offset
  * @clk_data: holds clock related data like clk_hw* and number of clocks
  */
 struct samsung_clk_provider {
 	void __iomem *reg_base;
 	struct device *dev;
+	struct regmap *sysreg;
 	spinlock_t lock;
+	bool auto_clock_gate;
+	u32 gate_dbg_offset;
+	u32 option_offset;
+	u32 drcg_offset;
+	u32 memclk_offset;
 	/* clk_data must be the last entry due to variable length 'hws' array */
 	struct clk_hw_onecell_data clk_data;
 };
@@ -310,6 +323,7 @@ struct samsung_cpu_clock {
 struct samsung_clock_reg_cache {
 	struct list_head node;
 	void __iomem *reg_base;
+	struct regmap *sysreg;
 	struct samsung_clk_reg_dump *rdump;
 	unsigned int rd_num;
 	const struct samsung_clk_reg_dump *rsuspend;
@@ -338,7 +352,14 @@ struct samsung_clock_reg_cache {
  * @suspend_regs: list of clock registers to set before suspend
  * @nr_suspend_regs: count of clock registers in @suspend_regs
  * @clk_name: name of the parent clock needed for CMU register access
+ * @sysreg_clk_regs: list of sysreg clock registers
+ * @nr_sysreg_clk_regs: count of clock registers in @sysreg_clk_regs
  * @manual_plls: Enable manual control for PLL clocks
+ * @auto_clock_gate: enable auto clock mode for all components in CMU
+ * @gate_dbg_offset: gate debug reg offset. Used by all gates in auto clk mode
+ * @option_offset: option reg offset. Enables auto clk mode for entire CMU
+ * @drcg_offset: dynamic root clk gate enable register offset
+ * @memclk_offset: memclk enable register offset
  */
 struct samsung_cmu_info {
 	const struct samsung_pll_clock *pll_clks;
@@ -364,8 +385,16 @@ struct samsung_cmu_info {
 	unsigned int nr_suspend_regs;
 	const char *clk_name;
 
+	const unsigned long *sysreg_clk_regs;
+	unsigned int nr_sysreg_clk_regs;
+
 	/* ARM64 Exynos CMUs */
 	bool manual_plls;
+	bool auto_clock_gate;
+	u32 gate_dbg_offset;
+	u32 option_offset;
+	u32 drcg_offset;
+	u32 memclk_offset;
 };
 
 struct samsung_clk_provider *samsung_clk_init(struct device *dev,
@@ -415,6 +444,7 @@ struct samsung_clk_provider *samsung_cmu_register_one(
 
 #ifdef CONFIG_PM_SLEEP
 void samsung_clk_extended_sleep_init(void __iomem *reg_base,
+			struct regmap *sysreg,
 			const unsigned long *rdump,
 			unsigned long nr_rdump,
 			const struct samsung_clk_reg_dump *rsuspend,
@@ -426,17 +456,32 @@ static inline void samsung_clk_extended_sleep_init(void __iomem *reg_base,
 			const struct samsung_clk_reg_dump *rsuspend,
 			unsigned long nr_rsuspend) {}
 #endif
-#define samsung_clk_sleep_init(reg_base, rdump, nr_rdump) \
-	samsung_clk_extended_sleep_init(reg_base, rdump, nr_rdump, NULL, 0)
+#define samsung_clk_sleep_init(reg_base, sysreg, rdump, nr_rdump)	   \
+	samsung_clk_extended_sleep_init(reg_base, sysreg, rdump, nr_rdump, \
+					NULL, 0)
 
 void samsung_clk_save(void __iomem *base,
+			struct regmap *regmap,
 			struct samsung_clk_reg_dump *rd,
 			unsigned int num_regs);
 void samsung_clk_restore(void __iomem *base,
+			struct regmap *regmap,
 			const struct samsung_clk_reg_dump *rd,
 			unsigned int num_regs);
 struct samsung_clk_reg_dump *samsung_clk_alloc_reg_dump(
 			const unsigned long *rdump,
 			unsigned long nr_rdump);
 
+void samsung_en_dyn_root_clk_gating(struct device_node *np,
+				struct samsung_clk_provider *ctx,
+				const struct samsung_cmu_info *cmu);
+
+struct clk_hw *samsung_register_auto_gate(struct device *dev,
+		struct device_node *np, const char *name,
+		const char *parent_name, const struct clk_hw *parent_hw,
+		const struct clk_parent_data *parent_data,
+		unsigned long flags,
+		void __iomem *reg, u8 bit_idx,
+		u8 clk_gate_flags, spinlock_t *lock);
+
 #endif /* __SAMSUNG_CLK_H */

-- 
2.51.0.760.g7b8bcc2412-goog


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

* [PATCH 8/9] clk: samsung: gs101: Enable auto_clock_gate mode for each gs101 CMU
  2025-10-13 20:51 [PATCH 0/9] Implement hardware automatic clock gating (HWACG) for gs101 Peter Griffin
                   ` (6 preceding siblings ...)
  2025-10-13 20:51 ` [PATCH 7/9] clk: samsung: Implement automatic clock gating mode for CMUs Peter Griffin
@ 2025-10-13 20:51 ` Peter Griffin
  2025-10-21 19:47   ` Krzysztof Kozlowski
  2025-10-13 20:51 ` [PATCH 9/9] clk: samsung: gs101: remove CLK_IGNORE_UNUSED and CLK_IS_CRITICAL flags Peter Griffin
  8 siblings, 1 reply; 30+ messages in thread
From: Peter Griffin @ 2025-10-13 20:51 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar,
	André Draszik, Tudor Ambarus, Michael Turquette,
	Stephen Boyd, Sam Protsenko, Sylwester Nawrocki, Chanwoo Choi
  Cc: Will McVicker, Krzysztof Kozlowski, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-clk, Krzysztof Kozlowski,
	kernel-team, Peter Griffin

Enable auto clock mode, and define the additional fields which are used
when this mode is enabled.

/sys/kernel/debug/clk/clk_summary now reports approximately 308 running
clocks and 298 disabled clocks. Prior to this commit 586 clocks were
running and 17 disabled. To ensure compatability with older DTs the
resource size is checked and an error issued if the DT needs updating.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 drivers/clk/samsung/clk-gs101.c | 80 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/drivers/clk/samsung/clk-gs101.c b/drivers/clk/samsung/clk-gs101.c
index 70b26db9b95ad0b376d23f637c7683fbc8c8c600..baf41ae6c9e2480cb83531acf7eae190c6aff819 100644
--- a/drivers/clk/samsung/clk-gs101.c
+++ b/drivers/clk/samsung/clk-gs101.c
@@ -9,6 +9,7 @@
 #include <linux/clk-provider.h>
 #include <linux/mod_devicetable.h>
 #include <linux/of.h>
+#include <linux/of_address.h>
 #include <linux/platform_device.h>
 
 #include <dt-bindings/clock/google,gs101.h>
@@ -17,6 +18,8 @@
 #include "clk-exynos-arm64.h"
 #include "clk-pll.h"
 
+int check_cmu_res_size(struct device_node *np);
+
 /* NOTE: Must be equal to the last clock ID increased by one */
 #define CLKS_NR_TOP	(CLK_GOUT_CMU_TPU_UART + 1)
 #define CLKS_NR_APM	(CLK_APM_PLL_DIV16_APM + 1)
@@ -26,6 +29,10 @@
 #define CLKS_NR_PERIC0	(CLK_GOUT_PERIC0_SYSREG_PERIC0_PCLK + 1)
 #define CLKS_NR_PERIC1	(CLK_GOUT_PERIC1_SYSREG_PERIC1_PCLK + 1)
 
+#define GS101_GATE_DBG_OFFSET 0x4000
+#define GS101_DRCG_EN_OFFSET  0x104
+#define GS101_MEMCLK_OFFSET   0x108
+
 /* ---- CMU_TOP ------------------------------------------------------------- */
 
 /* Register Offset definitions for CMU_TOP (0x1e080000) */
@@ -1433,6 +1440,9 @@ static const struct samsung_cmu_info top_cmu_info __initconst = {
 	.nr_clk_ids		= CLKS_NR_TOP,
 	.clk_regs		= cmu_top_clk_regs,
 	.nr_clk_regs		= ARRAY_SIZE(cmu_top_clk_regs),
+	.auto_clock_gate	= true,
+	.gate_dbg_offset	= GS101_GATE_DBG_OFFSET,
+	.option_offset		= CMU_CMU_TOP_CONTROLLER_OPTION,
 };
 
 static void __init gs101_cmu_top_init(struct device_node *np)
@@ -1900,6 +1910,11 @@ static const struct samsung_gate_clock apm_gate_clks[] __initconst = {
 	     CLK_CON_GAT_GOUT_BLK_APM_UID_XIU_DP_APM_IPCLKPORT_ACLK, 21, CLK_IS_CRITICAL, 0),
 };
 
+static const unsigned long dcrg_memclk_sysreg[] __initconst = {
+	GS101_DRCG_EN_OFFSET,
+	GS101_MEMCLK_OFFSET,
+};
+
 static const struct samsung_cmu_info apm_cmu_info __initconst = {
 	.mux_clks		= apm_mux_clks,
 	.nr_mux_clks		= ARRAY_SIZE(apm_mux_clks),
@@ -1912,6 +1927,12 @@ static const struct samsung_cmu_info apm_cmu_info __initconst = {
 	.nr_clk_ids		= CLKS_NR_APM,
 	.clk_regs		= apm_clk_regs,
 	.nr_clk_regs		= ARRAY_SIZE(apm_clk_regs),
+	.sysreg_clk_regs	= dcrg_memclk_sysreg,
+	.nr_sysreg_clk_regs	= ARRAY_SIZE(dcrg_memclk_sysreg),
+	.auto_clock_gate	= true,
+	.gate_dbg_offset	= GS101_GATE_DBG_OFFSET,
+	.drcg_offset		= GS101_DRCG_EN_OFFSET,
+	.memclk_offset		= GS101_MEMCLK_OFFSET,
 };
 
 /* ---- CMU_HSI0 ------------------------------------------------------------ */
@@ -2375,7 +2396,14 @@ static const struct samsung_cmu_info hsi0_cmu_info __initconst = {
 	.nr_clk_ids		= CLKS_NR_HSI0,
 	.clk_regs		= hsi0_clk_regs,
 	.nr_clk_regs		= ARRAY_SIZE(hsi0_clk_regs),
+	.sysreg_clk_regs	= dcrg_memclk_sysreg,
+	.nr_sysreg_clk_regs	= ARRAY_SIZE(dcrg_memclk_sysreg),
 	.clk_name		= "bus",
+	.auto_clock_gate        = true,
+	.gate_dbg_offset        = GS101_GATE_DBG_OFFSET,
+	.option_offset		= HSI0_CMU_HSI0_CONTROLLER_OPTION,
+	.drcg_offset		= GS101_DRCG_EN_OFFSET,
+	.memclk_offset		= GS101_MEMCLK_OFFSET,
 };
 
 /* ---- CMU_HSI2 ------------------------------------------------------------ */
@@ -2863,7 +2891,14 @@ static const struct samsung_cmu_info hsi2_cmu_info __initconst = {
 	.nr_clk_ids		= CLKS_NR_HSI2,
 	.clk_regs		= cmu_hsi2_clk_regs,
 	.nr_clk_regs		= ARRAY_SIZE(cmu_hsi2_clk_regs),
+	.sysreg_clk_regs	= dcrg_memclk_sysreg,
+	.nr_sysreg_clk_regs	= ARRAY_SIZE(dcrg_memclk_sysreg),
 	.clk_name		= "bus",
+	.auto_clock_gate        = true,
+	.gate_dbg_offset        = GS101_GATE_DBG_OFFSET,
+	.option_offset		= HSI2_CMU_HSI2_CONTROLLER_OPTION,
+	.drcg_offset		= GS101_DRCG_EN_OFFSET,
+	.memclk_offset		= GS101_MEMCLK_OFFSET,
 };
 
 /* ---- CMU_MISC ------------------------------------------------------------ */
@@ -3423,11 +3458,37 @@ static const struct samsung_cmu_info misc_cmu_info __initconst = {
 	.nr_clk_ids		= CLKS_NR_MISC,
 	.clk_regs		= misc_clk_regs,
 	.nr_clk_regs		= ARRAY_SIZE(misc_clk_regs),
+	.sysreg_clk_regs	= dcrg_memclk_sysreg,
+	.nr_sysreg_clk_regs	= ARRAY_SIZE(dcrg_memclk_sysreg),
 	.clk_name		= "bus",
+	.auto_clock_gate	= true,
+	.gate_dbg_offset	= GS101_GATE_DBG_OFFSET,
+	.option_offset		= MISC_CMU_MISC_CONTROLLER_OPTION,
+	.drcg_offset		= GS101_DRCG_EN_OFFSET,
+	.memclk_offset		= GS101_MEMCLK_OFFSET,
 };
 
+/* for old DT compatbility with incorrect CMU size*/
+int check_cmu_res_size(struct device_node *np)
+{
+	struct resource res;
+	resource_size_t size;
+
+	if (of_address_to_resource(np, 0, &res))
+		return -ENODEV;
+
+	size = resource_size(&res);
+	if (size != 0x10000) {
+		pr_warn("%pOF: resource to small. Please update your DT\n", np);
+		return -ENODEV;
+	}
+	return 0;
+}
+
 static void __init gs101_cmu_misc_init(struct device_node *np)
 {
+	if (check_cmu_res_size(np))
+		return;
 	exynos_arm64_register_cmu(NULL, np, &misc_cmu_info);
 }
 
@@ -4010,6 +4071,10 @@ static const struct samsung_gate_clock peric0_gate_clks[] __initconst = {
 	     21, 0, 0),
 };
 
+static const unsigned long dcrg_sysreg[] __initconst = {
+	GS101_DRCG_EN_OFFSET,
+};
+
 static const struct samsung_cmu_info peric0_cmu_info __initconst = {
 	.mux_clks		= peric0_mux_clks,
 	.nr_mux_clks		= ARRAY_SIZE(peric0_mux_clks),
@@ -4020,7 +4085,13 @@ static const struct samsung_cmu_info peric0_cmu_info __initconst = {
 	.nr_clk_ids		= CLKS_NR_PERIC0,
 	.clk_regs		= peric0_clk_regs,
 	.nr_clk_regs		= ARRAY_SIZE(peric0_clk_regs),
+	.sysreg_clk_regs	= dcrg_sysreg,
+	.nr_sysreg_clk_regs	= ARRAY_SIZE(dcrg_sysreg),
 	.clk_name		= "bus",
+	.auto_clock_gate        = true,
+	.gate_dbg_offset        = GS101_GATE_DBG_OFFSET,
+	.option_offset		= PERIC0_CMU_PERIC0_CONTROLLER_OPTION,
+	.drcg_offset		= GS101_DRCG_EN_OFFSET,
 };
 
 /* ---- CMU_PERIC1 ---------------------------------------------------------- */
@@ -4368,7 +4439,13 @@ static const struct samsung_cmu_info peric1_cmu_info __initconst = {
 	.nr_clk_ids		= CLKS_NR_PERIC1,
 	.clk_regs		= peric1_clk_regs,
 	.nr_clk_regs		= ARRAY_SIZE(peric1_clk_regs),
+	.sysreg_clk_regs	= dcrg_sysreg,
+	.nr_sysreg_clk_regs	= ARRAY_SIZE(dcrg_sysreg),
 	.clk_name		= "bus",
+	.auto_clock_gate        = true,
+	.gate_dbg_offset        = GS101_GATE_DBG_OFFSET,
+	.option_offset		= PERIC1_CMU_PERIC1_CONTROLLER_OPTION,
+	.drcg_offset		= GS101_DRCG_EN_OFFSET,
 };
 
 /* ---- platform_driver ----------------------------------------------------- */
@@ -4378,6 +4455,9 @@ static int __init gs101_cmu_probe(struct platform_device *pdev)
 	const struct samsung_cmu_info *info;
 	struct device *dev = &pdev->dev;
 
+	if (check_cmu_res_size(dev->of_node))
+		return -ENODEV;
+
 	info = of_device_get_match_data(dev);
 	exynos_arm64_register_cmu(dev, dev->of_node, info);
 

-- 
2.51.0.760.g7b8bcc2412-goog


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

* [PATCH 9/9] clk: samsung: gs101: remove CLK_IGNORE_UNUSED and CLK_IS_CRITICAL flags
  2025-10-13 20:51 [PATCH 0/9] Implement hardware automatic clock gating (HWACG) for gs101 Peter Griffin
                   ` (7 preceding siblings ...)
  2025-10-13 20:51 ` [PATCH 8/9] clk: samsung: gs101: Enable auto_clock_gate mode for each gs101 CMU Peter Griffin
@ 2025-10-13 20:51 ` Peter Griffin
  8 siblings, 0 replies; 30+ messages in thread
From: Peter Griffin @ 2025-10-13 20:51 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar,
	André Draszik, Tudor Ambarus, Michael Turquette,
	Stephen Boyd, Sam Protsenko, Sylwester Nawrocki, Chanwoo Choi
  Cc: Will McVicker, Krzysztof Kozlowski, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-clk, Krzysztof Kozlowski,
	kernel-team, Peter Griffin

Now each CMU is in automatic mode these flags are no longer necessary. All
unused clocks are automatically gated & ungated by hardware as
required.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 drivers/clk/samsung/clk-gs101.c | 87 +++++++++++++++++------------------------
 1 file changed, 35 insertions(+), 52 deletions(-)

diff --git a/drivers/clk/samsung/clk-gs101.c b/drivers/clk/samsung/clk-gs101.c
index baf41ae6c9e2480cb83531acf7eae190c6aff819..d01c94994d86bc27d344969c33955da63ed0e4a1 100644
--- a/drivers/clk/samsung/clk-gs101.c
+++ b/drivers/clk/samsung/clk-gs101.c
@@ -1901,13 +1901,13 @@ static const struct samsung_gate_clock apm_gate_clks[] __initconst = {
 	     CLK_CON_GAT_GOUT_BLK_APM_UID_UASC_P_APM_IPCLKPORT_ACLK, 21, CLK_IS_CRITICAL, 0),
 	GATE(CLK_GOUT_APM_UASC_P_APM_PCLK,
 	     "gout_apm_uasc_p_apm_pclk", "gout_apm_func",
-	     CLK_CON_GAT_GOUT_BLK_APM_UID_UASC_P_APM_IPCLKPORT_PCLK, 21, CLK_IS_CRITICAL, 0),
+	     CLK_CON_GAT_GOUT_BLK_APM_UID_UASC_P_APM_IPCLKPORT_PCLK, 21, 0, 0),
 	GATE(CLK_GOUT_APM_WDT_APM_PCLK,
 	     "gout_apm_wdt_apm_pclk", "gout_apm_func",
 	     CLK_CON_GAT_GOUT_BLK_APM_UID_WDT_APM_IPCLKPORT_PCLK, 21, 0, 0),
 	GATE(CLK_GOUT_APM_XIU_DP_APM_ACLK,
 	     "gout_apm_xiu_dp_apm_aclk", "gout_apm_func",
-	     CLK_CON_GAT_GOUT_BLK_APM_UID_XIU_DP_APM_IPCLKPORT_ACLK, 21, CLK_IS_CRITICAL, 0),
+	     CLK_CON_GAT_GOUT_BLK_APM_UID_XIU_DP_APM_IPCLKPORT_ACLK, 21, 0, 0),
 };
 
 static const unsigned long dcrg_memclk_sysreg[] __initconst = {
@@ -2211,11 +2211,10 @@ static const struct samsung_div_clock hsi0_div_clks[] __initconst = {
 };
 
 static const struct samsung_gate_clock hsi0_gate_clks[] __initconst = {
-	/* TODO: should have a driver for this */
 	GATE(CLK_GOUT_HSI0_PCLK,
 	     "gout_hsi0_hsi0_pclk", "mout_hsi0_bus",
 	     CLK_CON_GAT_CLK_BLK_HSI0_UID_HSI0_CMU_HSI0_IPCLKPORT_PCLK,
-	     21, CLK_IGNORE_UNUSED, 0),
+	     21, 0, 0),
 	GATE(CLK_GOUT_HSI0_USB31DRD_I_USB31DRD_SUSPEND_CLK_26,
 	     "gout_hsi0_usb31drd_i_usb31drd_suspend_clk_26",
 	     "mout_hsi0_usb20_ref",
@@ -2252,16 +2251,14 @@ static const struct samsung_gate_clock hsi0_gate_clks[] __initconst = {
 	     "gout_hsi0_lhm_axi_p_aochsi0_i_clk", "mout_hsi0_bus",
 	     CLK_CON_GAT_GOUT_BLK_HSI0_UID_LHM_AXI_P_AOCHSI0_IPCLKPORT_I_CLK,
 	     21, 0, 0),
-	/* TODO: should have a driver for this */
 	GATE(CLK_GOUT_HSI0_LHM_AXI_P_HSI0_I_CLK,
 	     "gout_hsi0_lhm_axi_p_hsi0_i_clk", "mout_hsi0_bus",
 	     CLK_CON_GAT_GOUT_BLK_HSI0_UID_LHM_AXI_P_HSI0_IPCLKPORT_I_CLK,
-	     21, CLK_IGNORE_UNUSED, 0),
-	/* TODO: should have a driver for this */
+	     21, 0, 0),
 	GATE(CLK_GOUT_HSI0_LHS_ACEL_D_HSI0_I_CLK,
 	     "gout_hsi0_lhs_acel_d_hsi0_i_clk", "mout_hsi0_bus",
 	     CLK_CON_GAT_GOUT_BLK_HSI0_UID_LHS_ACEL_D_HSI0_IPCLKPORT_I_CLK,
-	     21, CLK_IGNORE_UNUSED, 0),
+	     21, 0, 0),
 	GATE(CLK_GOUT_HSI0_LHS_AXI_D_HSI0AOC_I_CLK,
 	     "gout_hsi0_lhs_axi_d_hsi0aoc_i_clk", "mout_hsi0_bus",
 	     CLK_CON_GAT_GOUT_BLK_HSI0_UID_LHS_AXI_D_HSI0AOC_IPCLKPORT_I_CLK,
@@ -2286,21 +2283,18 @@ static const struct samsung_gate_clock hsi0_gate_clks[] __initconst = {
 	     "gout_hsi0_clk_hsi0_bus_clk", "mout_hsi0_bus",
 	     CLK_CON_GAT_GOUT_BLK_HSI0_UID_RSTNSYNC_CLK_HSI0_BUS_IPCLKPORT_CLK,
 	     21, 0, 0),
-	/* TODO: should have a driver for this */
 	GATE(CLK_GOUT_HSI0_SSMT_USB_ACLK,
 	     "gout_hsi0_ssmt_usb_aclk", "mout_hsi0_bus",
 	     CLK_CON_GAT_GOUT_BLK_HSI0_UID_SSMT_USB_IPCLKPORT_ACLK,
-	     21, CLK_IGNORE_UNUSED, 0),
-	/* TODO: should have a driver for this */
+	     21, 0, 0),
 	GATE(CLK_GOUT_HSI0_SSMT_USB_PCLK,
 	     "gout_hsi0_ssmt_usb_pclk", "mout_hsi0_bus",
 	     CLK_CON_GAT_GOUT_BLK_HSI0_UID_SSMT_USB_IPCLKPORT_PCLK,
-	     21, CLK_IGNORE_UNUSED, 0),
-	/* TODO: should have a driver for this */
+	     21, 0, 0),
 	GATE(CLK_GOUT_HSI0_SYSMMU_USB_CLK_S2,
 	     "gout_hsi0_sysmmu_usb_clk_s2", "mout_hsi0_bus",
 	     CLK_CON_GAT_GOUT_BLK_HSI0_UID_SYSMMU_USB_IPCLKPORT_CLK_S2,
-	     21, CLK_IGNORE_UNUSED, 0),
+	     21, 0, 0),
 	GATE(CLK_GOUT_HSI0_SYSREG_HSI0_PCLK,
 	     "gout_hsi0_sysreg_hsi0_pclk", "mout_hsi0_bus",
 	     CLK_CON_GAT_GOUT_BLK_HSI0_UID_SYSREG_HSI0_IPCLKPORT_PCLK,
@@ -2358,21 +2352,18 @@ static const struct samsung_gate_clock hsi0_gate_clks[] __initconst = {
 	     "gout_hsi0_usb31drd_usbdpphy_udbg_i_apb_pclk", "mout_hsi0_bus",
 	     CLK_CON_GAT_GOUT_BLK_HSI0_UID_USB31DRD_IPCLKPORT_USBDPPHY_UDBG_I_APB_PCLK,
 	     21, 0, 0),
-	/* TODO: should have a driver for this */
 	GATE(CLK_GOUT_HSI0_XIU_D0_HSI0_ACLK,
 	     "gout_hsi0_xiu_d0_hsi0_aclk", "mout_hsi0_bus",
 	     CLK_CON_GAT_GOUT_BLK_HSI0_UID_XIU_D0_HSI0_IPCLKPORT_ACLK,
-	     21, CLK_IGNORE_UNUSED, 0),
-	/* TODO: should have a driver for this */
+	     21, 0, 0),
 	GATE(CLK_GOUT_HSI0_XIU_D1_HSI0_ACLK,
 	     "gout_hsi0_xiu_d1_hsi0_aclk", "mout_hsi0_bus",
 	     CLK_CON_GAT_GOUT_BLK_HSI0_UID_XIU_D1_HSI0_IPCLKPORT_ACLK,
-	     21, CLK_IGNORE_UNUSED, 0),
-	/* TODO: should have a driver for this */
+	     21, 0, 0),
 	GATE(CLK_GOUT_HSI0_XIU_P_HSI0_ACLK,
 	     "gout_hsi0_xiu_p_hsi0_aclk", "mout_hsi0_bus",
 	     CLK_CON_GAT_GOUT_BLK_HSI0_UID_XIU_P_HSI0_IPCLKPORT_ACLK,
-	     21, CLK_IGNORE_UNUSED, 0),
+	     21, 0, 0),
 };
 
 static const struct samsung_fixed_rate_clock hsi0_fixed_clks[] __initconst = {
@@ -2677,22 +2668,19 @@ static const struct samsung_gate_clock hsi2_gate_clks[] __initconst = {
 	GATE(CLK_GOUT_HSI2_GPIO_HSI2_PCLK,
 	     "gout_hsi2_gpio_hsi2_pclk", "mout_hsi2_bus_user",
 	     CLK_CON_GAT_GOUT_BLK_HSI2_UID_GPIO_HSI2_IPCLKPORT_PCLK, 21,
-	     CLK_IGNORE_UNUSED, 0),
-	/* Disabling this clock makes the system hang. Mark the clock as critical. */
+	     0, 0),
 	GATE(CLK_GOUT_HSI2_HSI2_CMU_HSI2_PCLK,
 	     "gout_hsi2_hsi2_cmu_hsi2_pclk", "mout_hsi2_bus_user",
 	     CLK_CON_GAT_GOUT_BLK_HSI2_UID_HSI2_CMU_HSI2_IPCLKPORT_PCLK,
-	     21, CLK_IS_CRITICAL, 0),
-	/* Disabling this clock makes the system hang. Mark the clock as critical. */
+	     21, 0, 0),
 	GATE(CLK_GOUT_HSI2_LHM_AXI_P_HSI2_I_CLK,
 	     "gout_hsi2_lhm_axi_p_hsi2_i_clk", "mout_hsi2_bus_user",
 	     CLK_CON_GAT_GOUT_BLK_HSI2_UID_LHM_AXI_P_HSI2_IPCLKPORT_I_CLK,
-	     21, CLK_IS_CRITICAL, 0),
-	/* TODO: should have a driver for this */
+	     21, 0, 0),
 	GATE(CLK_GOUT_HSI2_LHS_ACEL_D_HSI2_I_CLK,
 	     "gout_hsi2_lhs_acel_d_hsi2_i_clk", "mout_hsi2_bus_user",
 	     CLK_CON_GAT_GOUT_BLK_HSI2_UID_LHS_ACEL_D_HSI2_IPCLKPORT_I_CLK,
-	     21, CLK_IGNORE_UNUSED, 0),
+	     21, 0, 0),
 	GATE(CLK_GOUT_HSI2_MMC_CARD_I_ACLK,
 	     "gout_hsi2_mmc_card_i_aclk", "mout_hsi2_bus_user",
 	     CLK_CON_GAT_GOUT_BLK_HSI2_UID_MMC_CARD_IPCLKPORT_I_ACLK,
@@ -2795,38 +2783,35 @@ static const struct samsung_gate_clock hsi2_gate_clks[] __initconst = {
 	GATE(CLK_GOUT_HSI2_QE_UFS_EMBD_HSI2_ACLK,
 	     "gout_hsi2_qe_ufs_embd_hsi2_aclk", "mout_hsi2_bus_user",
 	     CLK_CON_GAT_GOUT_BLK_HSI2_UID_QE_UFS_EMBD_HSI2_IPCLKPORT_ACLK,
-	     21, CLK_IS_CRITICAL, 0),
+	     21, 0, 0),
 	GATE(CLK_GOUT_HSI2_QE_UFS_EMBD_HSI2_PCLK,
 	     "gout_hsi2_qe_ufs_embd_hsi2_pclk", "mout_hsi2_bus_user",
 	     CLK_CON_GAT_GOUT_BLK_HSI2_UID_QE_UFS_EMBD_HSI2_IPCLKPORT_PCLK,
-	     21, CLK_IS_CRITICAL, 0),
+	     21, 0, 0),
 	GATE(CLK_GOUT_HSI2_CLK_HSI2_BUS_CLK,
 	     "gout_hsi2_clk_hsi2_bus_clk", "mout_hsi2_bus_user",
 	     CLK_CON_GAT_GOUT_BLK_HSI2_UID_RSTNSYNC_CLK_HSI2_BUS_IPCLKPORT_CLK,
-	     21, CLK_IS_CRITICAL, 0),
+	     21, 0, 0),
 	GATE(CLK_GOUT_HSI2_CLK_HSI2_OSCCLK_CLK,
 	     "gout_hsi2_clk_hsi2_oscclk_clk", "oscclk",
 	     CLK_CON_GAT_GOUT_BLK_HSI2_UID_RSTNSYNC_CLK_HSI2_OSCCLK_IPCLKPORT_CLK,
 	     21, 0, 0),
-	/* TODO: should have a driver for this */
 	GATE(CLK_GOUT_HSI2_SSMT_HSI2_ACLK,
 	     "gout_hsi2_ssmt_hsi2_aclk", "mout_hsi2_bus_user",
 	     CLK_CON_GAT_GOUT_BLK_HSI2_UID_SSMT_HSI2_IPCLKPORT_ACLK,
-	     21, CLK_IGNORE_UNUSED, 0),
-	/* TODO: should have a driver for this */
+	     21, 0, 0),
 	GATE(CLK_GOUT_HSI2_SSMT_HSI2_PCLK,
 	     "gout_hsi2_ssmt_hsi2_pclk", "mout_hsi2_bus_user",
 	     CLK_CON_GAT_GOUT_BLK_HSI2_UID_SSMT_HSI2_IPCLKPORT_PCLK,
-	     21, CLK_IGNORE_UNUSED, 0),
-	/* TODO: should have a driver for this */
+	     21, 0, 0),
 	GATE(CLK_GOUT_HSI2_SYSMMU_HSI2_CLK_S2,
 	     "gout_hsi2_sysmmu_hsi2_clk_s2", "mout_hsi2_bus_user",
 	     CLK_CON_GAT_GOUT_BLK_HSI2_UID_SYSMMU_HSI2_IPCLKPORT_CLK_S2,
-	     21, CLK_IGNORE_UNUSED, 0),
+	     21, 0, 0),
 	GATE(CLK_GOUT_HSI2_SYSREG_HSI2_PCLK,
 	     "gout_hsi2_sysreg_hsi2_pclk", "mout_hsi2_bus_user",
 	     CLK_CON_GAT_GOUT_BLK_HSI2_UID_SYSREG_HSI2_IPCLKPORT_PCLK,
-	     21, CLK_IS_CRITICAL, 0),
+	     21, 0, 0),
 	GATE(CLK_GOUT_HSI2_UASC_PCIE_GEN4A_DBI_1_ACLK,
 	     "gout_hsi2_uasc_pcie_gen4a_dbi_1_aclk", "mout_hsi2_bus_user",
 	     CLK_CON_GAT_GOUT_BLK_HSI2_UID_UASC_PCIE_GEN4A_DBI_1_IPCLKPORT_ACLK,
@@ -2862,25 +2847,23 @@ static const struct samsung_gate_clock hsi2_gate_clks[] __initconst = {
 	GATE(CLK_GOUT_HSI2_UFS_EMBD_I_ACLK,
 	     "gout_hsi2_ufs_embd_i_aclk", "mout_hsi2_bus_user",
 	     CLK_CON_GAT_GOUT_BLK_HSI2_UID_UFS_EMBD_IPCLKPORT_I_ACLK,
-	     21, CLK_IS_CRITICAL, 0),
+	     21, 0, 0),
 	GATE(CLK_GOUT_HSI2_UFS_EMBD_I_CLK_UNIPRO,
 	     "gout_hsi2_ufs_embd_i_clk_unipro", "mout_hsi2_ufs_embd_user",
 	     CLK_CON_GAT_GOUT_BLK_HSI2_UID_UFS_EMBD_IPCLKPORT_I_CLK_UNIPRO,
-	     21, CLK_IS_CRITICAL, 0),
+	     21, 0, 0),
 	GATE(CLK_GOUT_HSI2_UFS_EMBD_I_FMP_CLK,
 	     "gout_hsi2_ufs_embd_i_fmp_clk", "mout_hsi2_bus_user",
 	     CLK_CON_GAT_GOUT_BLK_HSI2_UID_UFS_EMBD_IPCLKPORT_I_FMP_CLK,
-	     21, CLK_IS_CRITICAL, 0),
-	/* TODO: should have a driver for this */
+	     21, 0, 0),
 	GATE(CLK_GOUT_HSI2_XIU_D_HSI2_ACLK,
 	     "gout_hsi2_xiu_d_hsi2_aclk", "mout_hsi2_bus_user",
 	     CLK_CON_GAT_GOUT_BLK_HSI2_UID_XIU_D_HSI2_IPCLKPORT_ACLK,
-	     21, CLK_IGNORE_UNUSED, 0),
-	/* TODO: should have a driver for this */
+	     21, 0, 0),
 	GATE(CLK_GOUT_HSI2_XIU_P_HSI2_ACLK,
 	     "gout_hsi2_xiu_p_hsi2_aclk", "mout_hsi2_bus_user",
 	     CLK_CON_GAT_GOUT_BLK_HSI2_UID_XIU_P_HSI2_IPCLKPORT_ACLK,
-	     21, CLK_IGNORE_UNUSED, 0),
+	     21, 0, 0),
 };
 
 static const struct samsung_cmu_info hsi2_cmu_info __initconst = {
@@ -3849,7 +3832,7 @@ static const struct samsung_gate_clock peric0_gate_clks[] __initconst = {
 	GATE(CLK_GOUT_PERIC0_PERIC0_CMU_PERIC0_PCLK,
 	     "gout_peric0_peric0_cmu_peric0_pclk", "mout_peric0_bus_user",
 	     CLK_CON_GAT_CLK_BLK_PERIC0_UID_PERIC0_CMU_PERIC0_IPCLKPORT_PCLK,
-	     21, CLK_IS_CRITICAL, 0),
+	     21, 0, 0),
 	GATE(CLK_GOUT_PERIC0_CLK_PERIC0_OSCCLK_CLK,
 	     "gout_peric0_clk_peric0_oscclk_clk", "oscclk",
 	     CLK_CON_GAT_CLK_BLK_PERIC0_UID_RSTNSYNC_CLK_PERIC0_OSCCLK_IPCLKPORT_CLK,
@@ -3865,12 +3848,12 @@ static const struct samsung_gate_clock peric0_gate_clks[] __initconst = {
 	GATE(CLK_GOUT_PERIC0_GPIO_PERIC0_PCLK,
 	     "gout_peric0_gpio_peric0_pclk", "mout_peric0_bus_user",
 	     CLK_CON_GAT_GOUT_BLK_PERIC0_UID_GPIO_PERIC0_IPCLKPORT_PCLK,
-	     21, CLK_IGNORE_UNUSED, 0),
+	     21, 0, 0),
 	/* Disabling this clock makes the system hang. Mark the clock as critical. */
 	GATE(CLK_GOUT_PERIC0_LHM_AXI_P_PERIC0_I_CLK,
 	     "gout_peric0_lhm_axi_p_peric0_i_clk", "mout_peric0_bus_user",
 	     CLK_CON_GAT_GOUT_BLK_PERIC0_UID_LHM_AXI_P_PERIC0_IPCLKPORT_I_CLK,
-	     21, CLK_IS_CRITICAL, 0),
+	     21, 0, 0),
 	GATE(CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_0,
 	     "gout_peric0_peric0_top0_ipclk_0", "dout_peric0_usi1_usi",
 	     CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP0_IPCLKPORT_IPCLK_0,
@@ -4003,7 +3986,7 @@ static const struct samsung_gate_clock peric0_gate_clks[] __initconst = {
 	GATE(CLK_GOUT_PERIC0_PERIC0_TOP1_IPCLK_0,
 	     "gout_peric0_peric0_top1_ipclk_0", "dout_peric0_usi0_uart",
 	     CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP1_IPCLKPORT_IPCLK_0,
-	     21, CLK_IS_CRITICAL, 0),
+	     21, 0, 0),
 	GATE(CLK_GOUT_PERIC0_PERIC0_TOP1_IPCLK_2,
 	     "gout_peric0_peric0_top1_ipclk_2", "dout_peric0_usi14_usi",
 	     CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP1_IPCLKPORT_IPCLK_2,
@@ -4012,7 +3995,7 @@ static const struct samsung_gate_clock peric0_gate_clks[] __initconst = {
 	GATE(CLK_GOUT_PERIC0_PERIC0_TOP1_PCLK_0,
 	     "gout_peric0_peric0_top1_pclk_0", "mout_peric0_bus_user",
 	     CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP1_IPCLKPORT_PCLK_0,
-	     21, CLK_IS_CRITICAL, 0),
+	     21, 0, 0),
 	GATE(CLK_GOUT_PERIC0_PERIC0_TOP1_PCLK_2,
 	     "gout_peric0_peric0_top1_pclk_2", "mout_peric0_bus_user",
 	     CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP1_IPCLKPORT_PCLK_2,
@@ -4310,7 +4293,7 @@ static const struct samsung_gate_clock peric1_gate_clks[] __initconst = {
 	GATE(CLK_GOUT_PERIC1_PCLK,
 	     "gout_peric1_peric1_pclk", "mout_peric1_bus_user",
 	     CLK_CON_GAT_CLK_BLK_PERIC1_UID_PERIC1_CMU_PERIC1_IPCLKPORT_PCLK,
-	     21, CLK_IS_CRITICAL, 0),
+	     21, 0, 0),
 	GATE(CLK_GOUT_PERIC1_CLK_PERIC1_I3C_CLK,
 	     "gout_peric1_clk_peric1_i3c_clk", "dout_peric1_i3c",
 	     CLK_CON_GAT_CLK_BLK_PERIC1_UID_RSTNSYNC_CLK_PERIC1_I3C_IPCLKPORT_CLK,
@@ -4330,11 +4313,11 @@ static const struct samsung_gate_clock peric1_gate_clks[] __initconst = {
 	GATE(CLK_GOUT_PERIC1_GPIO_PERIC1_PCLK,
 	     "gout_peric1_gpio_peric1_pclk", "mout_peric1_bus_user",
 	     CLK_CON_GAT_GOUT_BLK_PERIC1_UID_GPIO_PERIC1_IPCLKPORT_PCLK,
-	     21, CLK_IGNORE_UNUSED, 0),
+	     21, 0, 0),
 	GATE(CLK_GOUT_PERIC1_LHM_AXI_P_PERIC1_I_CLK,
 	     "gout_peric1_lhm_axi_p_peric1_i_clk", "mout_peric1_bus_user",
 	     CLK_CON_GAT_GOUT_BLK_PERIC1_UID_LHM_AXI_P_PERIC1_IPCLKPORT_I_CLK,
-	     21, CLK_IS_CRITICAL, 0),
+	     21, 0, 0),
 	GATE(CLK_GOUT_PERIC1_PERIC1_TOP0_IPCLK_1,
 	     "gout_peric1_peric1_top0_ipclk_1", "dout_peric1_usi0_usi",
 	     CLK_CON_GAT_GOUT_BLK_PERIC1_UID_PERIC1_TOP0_IPCLKPORT_IPCLK_1,

-- 
2.51.0.760.g7b8bcc2412-goog


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

* Re: [PATCH 7/9] clk: samsung: Implement automatic clock gating mode for CMUs
  2025-10-13 20:51 ` [PATCH 7/9] clk: samsung: Implement automatic clock gating mode for CMUs Peter Griffin
@ 2025-10-14 14:46   ` kernel test robot
  2025-10-16  9:40   ` André Draszik
  2025-10-21 19:45   ` Krzysztof Kozlowski
  2 siblings, 0 replies; 30+ messages in thread
From: kernel test robot @ 2025-10-14 14:46 UTC (permalink / raw)
  To: Peter Griffin, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Alim Akhtar, André Draszik, Tudor Ambarus, Michael Turquette,
	Stephen Boyd, Sam Protsenko, Sylwester Nawrocki, Chanwoo Choi
  Cc: llvm, oe-kbuild-all, Will McVicker, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-clk, kernel-team,
	Peter Griffin

Hi Peter,

kernel test robot noticed the following build errors:

[auto build test ERROR on 4a71531471926e3c391665ee9c42f4e0295a4585]

url:    https://github.com/intel-lab-lkp/linux/commits/Peter-Griffin/dt-bindings-soc-samsung-exynos-sysreg-add-gs101-hsi0-and-misc-compatibles/20251014-045559
base:   4a71531471926e3c391665ee9c42f4e0295a4585
patch link:    https://lore.kernel.org/r/20251013-automatic-clocks-v1-7-72851ee00300%40linaro.org
patch subject: [PATCH 7/9] clk: samsung: Implement automatic clock gating mode for CMUs
config: loongarch-randconfig-001-20251014 (https://download.01.org/0day-ci/archive/20251014/202510142228.IQJSNIFa-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251014/202510142228.IQJSNIFa-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510142228.IQJSNIFa-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

>> drivers/clk/samsung/clk.c:481:13: error: too many arguments to function call, expected 5, have 6
     478 |                 samsung_clk_extended_sleep_init(NULL, ctx->sysreg,
         |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     479 |                                                 cmu->sysreg_clk_regs,
     480 |                                                 cmu->nr_sysreg_clk_regs,
     481 |                                                 NULL, 0);
         |                                                       ^
   drivers/clk/samsung/clk.h:453:20: note: 'samsung_clk_extended_sleep_init' declared here
     453 | static inline void samsung_clk_extended_sleep_init(void __iomem *reg_base,
         |                    ^                               ~~~~~~~~~~~~~~~~~~~~~~~
     454 |                         const unsigned long *rdump,
         |                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~
     455 |                         unsigned long nr_rdump,
         |                         ~~~~~~~~~~~~~~~~~~~~~~~
     456 |                         const struct samsung_clk_reg_dump *rsuspend,
         |                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     457 |                         unsigned long nr_rsuspend) {}
         |                         ~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/clk/samsung/clk.c:511:23: error: too many arguments to function call, expected 5, have 6
     509 |                 samsung_clk_extended_sleep_init(reg_base, NULL,
         |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     510 |                         cmu->clk_regs, cmu->nr_clk_regs,
     511 |                         cmu->suspend_regs, cmu->nr_suspend_regs);
         |                                            ^~~~~~~~~~~~~~~~~~~~
   drivers/clk/samsung/clk.h:453:20: note: 'samsung_clk_extended_sleep_init' declared here
     453 | static inline void samsung_clk_extended_sleep_init(void __iomem *reg_base,
         |                    ^                               ~~~~~~~~~~~~~~~~~~~~~~~
     454 |                         const unsigned long *rdump,
         |                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~
     455 |                         unsigned long nr_rdump,
         |                         ~~~~~~~~~~~~~~~~~~~~~~~
     456 |                         const struct samsung_clk_reg_dump *rsuspend,
         |                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     457 |                         unsigned long nr_rsuspend) {}
         |                         ~~~~~~~~~~~~~~~~~~~~~~~~~
   2 errors generated.
--
>> Warning: drivers/clk/samsung/clk-exynos-arm64.c:249 function parameter 'init_clk_regs' not described in 'exynos_arm64_register_cmu_pm'


vim +481 drivers/clk/samsung/clk.c

   462	
   463	/* Enable Dynamic Root Clock Gating of bus components*/
   464	void samsung_en_dyn_root_clk_gating(struct device_node *np,
   465					    struct samsung_clk_provider *ctx,
   466					    const struct samsung_cmu_info *cmu)
   467	{
   468		if (ctx && !ctx->auto_clock_gate)
   469			return;
   470	
   471		ctx->sysreg = syscon_regmap_lookup_by_phandle(np, "samsung,sysreg");
   472		if (!IS_ERR_OR_NULL(ctx->sysreg)) {
   473			regmap_write(ctx->sysreg, ctx->drcg_offset, 0xffffffff);
   474			/* not every sysreg controller has memclk reg*/
   475			if (ctx->memclk_offset)
   476				regmap_write_bits(ctx->sysreg, ctx->memclk_offset, 0x1, 0x0);
   477	
   478			samsung_clk_extended_sleep_init(NULL, ctx->sysreg,
   479							cmu->sysreg_clk_regs,
   480							cmu->nr_sysreg_clk_regs,
 > 481							NULL, 0);
   482		} else {
   483			pr_warn("%pOF: Unable to get CMU sysreg\n", np);
   484			ctx->sysreg = NULL;
   485		}
   486	}
   487	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH 1/9] dt-bindings: soc: samsung: exynos-sysreg: add gs101 hsi0 and misc compatibles
  2025-10-13 20:51 ` [PATCH 1/9] dt-bindings: soc: samsung: exynos-sysreg: add gs101 hsi0 and misc compatibles Peter Griffin
@ 2025-10-15 18:31   ` Rob Herring (Arm)
  2025-10-16  5:44   ` André Draszik
  2025-10-21 19:27   ` (subset) " Krzysztof Kozlowski
  2 siblings, 0 replies; 30+ messages in thread
From: Rob Herring (Arm) @ 2025-10-15 18:31 UTC (permalink / raw)
  To: Peter Griffin
  Cc: linux-clk, devicetree, Sam Protsenko, Krzysztof Kozlowski,
	Tudor Ambarus, Will McVicker, Conor Dooley, Krzysztof Kozlowski,
	Sylwester Nawrocki, linux-samsung-soc, linux-arm-kernel,
	Krzysztof Kozlowski, kernel-team, linux-kernel, Chanwoo Choi,
	Alim Akhtar, Stephen Boyd, André Draszik, Michael Turquette


On Mon, 13 Oct 2025 21:51:30 +0100, Peter Griffin wrote:
> Add dedicated compatibles for gs101 hsi0 and misc sysreg controllers to the
> documentation.
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  .../devicetree/bindings/soc/samsung/samsung,exynos-sysreg.yaml        | 4 ++++
>  1 file changed, 4 insertions(+)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>


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

* Re: [PATCH 2/9] dt-bindings: clock: google,gs101-clock: add samsung,sysreg property as required
  2025-10-13 20:51 ` [PATCH 2/9] dt-bindings: clock: google,gs101-clock: add samsung,sysreg property as required Peter Griffin
@ 2025-10-15 18:32   ` Rob Herring (Arm)
  2025-10-16  5:53   ` André Draszik
  2025-10-21 19:22   ` Krzysztof Kozlowski
  2 siblings, 0 replies; 30+ messages in thread
From: Rob Herring (Arm) @ 2025-10-15 18:32 UTC (permalink / raw)
  To: Peter Griffin
  Cc: Alim Akhtar, Krzysztof Kozlowski, devicetree, Tudor Ambarus,
	André Draszik, linux-kernel, Sam Protsenko,
	linux-samsung-soc, linux-arm-kernel, Stephen Boyd,
	Michael Turquette, Conor Dooley, Chanwoo Choi, Sylwester Nawrocki,
	linux-clk, Krzysztof Kozlowski, kernel-team, Will McVicker,
	Krzysztof Kozlowski


On Mon, 13 Oct 2025 21:51:31 +0100, Peter Griffin wrote:
> Update the bindings documentation so that all CMUs (with the exception of
> gs101-cmu-top) have samsung,sysreg as a required property.
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  .../bindings/clock/google,gs101-clock.yaml         | 23 +++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>


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

* Re: [PATCH 1/9] dt-bindings: soc: samsung: exynos-sysreg: add gs101 hsi0 and misc compatibles
  2025-10-13 20:51 ` [PATCH 1/9] dt-bindings: soc: samsung: exynos-sysreg: add gs101 hsi0 and misc compatibles Peter Griffin
  2025-10-15 18:31   ` Rob Herring (Arm)
@ 2025-10-16  5:44   ` André Draszik
  2025-10-21 19:27   ` (subset) " Krzysztof Kozlowski
  2 siblings, 0 replies; 30+ messages in thread
From: André Draszik @ 2025-10-16  5:44 UTC (permalink / raw)
  To: Peter Griffin, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Alim Akhtar, Tudor Ambarus, Michael Turquette, Stephen Boyd,
	Sam Protsenko, Sylwester Nawrocki, Chanwoo Choi
  Cc: Will McVicker, Krzysztof Kozlowski, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-clk, Krzysztof Kozlowski,
	kernel-team

On Mon, 2025-10-13 at 21:51 +0100, Peter Griffin wrote:
> Add dedicated compatibles for gs101 hsi0 and misc sysreg controllers to the
> documentation.
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  .../devicetree/bindings/soc/samsung/samsung,exynos-sysreg.yaml        | 4 ++++
>  1 file changed, 4 insertions(+)

Reviewed-by: André Draszik <andre.draszik@linaro.org>

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

* Re: [PATCH 2/9] dt-bindings: clock: google,gs101-clock: add samsung,sysreg property as required
  2025-10-13 20:51 ` [PATCH 2/9] dt-bindings: clock: google,gs101-clock: add samsung,sysreg property as required Peter Griffin
  2025-10-15 18:32   ` Rob Herring (Arm)
@ 2025-10-16  5:53   ` André Draszik
  2025-10-21 19:22   ` Krzysztof Kozlowski
  2 siblings, 0 replies; 30+ messages in thread
From: André Draszik @ 2025-10-16  5:53 UTC (permalink / raw)
  To: Peter Griffin, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Alim Akhtar, Tudor Ambarus, Michael Turquette, Stephen Boyd,
	Sam Protsenko, Sylwester Nawrocki, Chanwoo Choi
  Cc: Will McVicker, Krzysztof Kozlowski, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-clk, Krzysztof Kozlowski,
	kernel-team

On Mon, 2025-10-13 at 21:51 +0100, Peter Griffin wrote:
> Update the bindings documentation so that all CMUs (with the exception of
> gs101-cmu-top) have samsung,sysreg as a required property.
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  .../bindings/clock/google,gs101-clock.yaml         | 23 +++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml b/Documentation/devicetree/bindings/clock/google,gs101-
> clock.yaml
> index caf442ead24bda57e531420d8a7d8de8713032ae..5cfe98d9ba895d5207fffc82f3fd55b602b4a2bb 100644
> --- a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml
> +++ b/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml
> @@ -49,6 +49,11 @@ properties:
>    reg:
>      maxItems: 1
>  
> +  samsung,sysreg:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      Phandle to system registers interface.
> +
>  required:
>    - compatible
>    - "#clock-cells"
> @@ -163,6 +168,22 @@ allOf:
>              - const: bus
>              - const: ip
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - google,gs101-cmu-apm
> +              - google,gs101-cmu-misc
> +              - google,gs101-hsi0
> +              - google,gs101-cmu-hsi2
> +              - google,gs101-cmu-peric0
> +              - google,gs101-cmu-peric1
> +
> +    then:
> +      required:
> +        - samsung,sysreg
> +
>  additionalProperties: false
>  
>  examples:
> @@ -172,7 +193,7 @@ examples:
>  
>      cmu_top: clock-controller@1e080000 {
>          compatible = "google,gs101-cmu-top";
> -        reg = <0x1e080000 0x8000>;
> +        reg = <0x1e080000 0x10000>;

This seems unrelated, or at least not mentioned in the commit message.

Cheers,
Andre'

>          #clock-cells = <1>;
>          clocks = <&ext_24_5m>;
>          clock-names = "oscclk";

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

* Re: [PATCH 3/9] arm64: dts: exynos: gs101: add sysreg_misc and sysreg_hsi0 nodes
  2025-10-13 20:51 ` [PATCH 3/9] arm64: dts: exynos: gs101: add sysreg_misc and sysreg_hsi0 nodes Peter Griffin
@ 2025-10-16  5:56   ` André Draszik
  2025-10-21 19:27   ` (subset) " Krzysztof Kozlowski
  1 sibling, 0 replies; 30+ messages in thread
From: André Draszik @ 2025-10-16  5:56 UTC (permalink / raw)
  To: Peter Griffin, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Alim Akhtar, Tudor Ambarus, Michael Turquette, Stephen Boyd,
	Sam Protsenko, Sylwester Nawrocki, Chanwoo Choi
  Cc: Will McVicker, Krzysztof Kozlowski, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-clk, Krzysztof Kozlowski,
	kernel-team

On Mon, 2025-10-13 at 21:51 +0100, Peter Griffin wrote:
> Add syscon DT node for the hsi0 and misc sysreg controllers. These will be
> referenced by their respective CMU nodes in future patchs.

s/patchs/patches :-)

> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  arch/arm64/boot/dts/exynos/google/gs101.dtsi | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

Reviewed-by: André Draszik <andre.draszik@linaro.org>

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

* Re: [PATCH 4/9] arm64: dts: exynos: gs101: fix clock module unit reg sizes
  2025-10-13 20:51 ` [PATCH 4/9] arm64: dts: exynos: gs101: fix clock module unit reg sizes Peter Griffin
@ 2025-10-16  8:22   ` André Draszik
  2025-10-21 19:27   ` (subset) " Krzysztof Kozlowski
  1 sibling, 0 replies; 30+ messages in thread
From: André Draszik @ 2025-10-16  8:22 UTC (permalink / raw)
  To: Peter Griffin, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Alim Akhtar, Tudor Ambarus, Michael Turquette, Stephen Boyd,
	Sam Protsenko, Sylwester Nawrocki, Chanwoo Choi
  Cc: Will McVicker, Krzysztof Kozlowski, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-clk, Krzysztof Kozlowski,
	kernel-team

On Mon, 2025-10-13 at 21:51 +0100, Peter Griffin wrote:
> The memory map lists each clock module unit as having a size of
> 0x10000. Additionally there are some undocumented registers in this region
> that need to be used for automatic clock gating mode. Some of those
> registers also need to be saved/restored on suspend & resume.
> 
> Fixes: 86124c76683e ("arm64: dts: exynos: gs101: enable cmu-hsi2 clock controller")
> Fixes: 4982a4a2092e ("arm64: dts: exynos: gs101: enable cmu-hsi0 clock controller")
> Fixes: 7d66d98b5bf3 ("arm64: dts: exynos: gs101: enable cmu-peric1 clock controller")
> Fixes: e62c706f3aa0 ("arm64: dts: exynos: gs101: enable cmu-peric0 clock controller")
> Fixes: ea89fdf24fd9 ("arm64: dts: exynos: google: Add initial Google gs101 SoC support")
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  arch/arm64/boot/dts/exynos/google/gs101.dtsi | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)

Reviewed-by: André Draszik <andre.draszik@linaro.org>

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

* Re: [PATCH 5/9] arm64: dts: exynos: gs101: fix sysreg_apm reg property
  2025-10-13 20:51 ` [PATCH 5/9] arm64: dts: exynos: gs101: fix sysreg_apm reg property Peter Griffin
@ 2025-10-16  8:23   ` André Draszik
  2025-10-21 19:27   ` (subset) " Krzysztof Kozlowski
  1 sibling, 0 replies; 30+ messages in thread
From: André Draszik @ 2025-10-16  8:23 UTC (permalink / raw)
  To: Peter Griffin, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Alim Akhtar, Tudor Ambarus, Michael Turquette, Stephen Boyd,
	Sam Protsenko, Sylwester Nawrocki, Chanwoo Choi
  Cc: Will McVicker, Krzysztof Kozlowski, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-clk, Krzysztof Kozlowski,
	kernel-team

On Mon, 2025-10-13 at 21:51 +0100, Peter Griffin wrote:
> Both the start address and size are incorrect for the apm_sysreg DT
> node. Update to match the TRM (rather than how it was defined downstream).
> 
> Fixes: ea89fdf24fd9 ("arm64: dts: exynos: google: Add initial Google gs101 SoC support")
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  arch/arm64/boot/dts/exynos/google/gs101.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: André Draszik <andre.draszik@linaro.org>

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

* Re: [PATCH 7/9] clk: samsung: Implement automatic clock gating mode for CMUs
  2025-10-13 20:51 ` [PATCH 7/9] clk: samsung: Implement automatic clock gating mode for CMUs Peter Griffin
  2025-10-14 14:46   ` kernel test robot
@ 2025-10-16  9:40   ` André Draszik
  2025-10-28  8:50     ` Peter Griffin
  2025-10-21 19:45   ` Krzysztof Kozlowski
  2 siblings, 1 reply; 30+ messages in thread
From: André Draszik @ 2025-10-16  9:40 UTC (permalink / raw)
  To: Peter Griffin, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Alim Akhtar, Tudor Ambarus, Michael Turquette, Stephen Boyd,
	Sam Protsenko, Sylwester Nawrocki, Chanwoo Choi
  Cc: Will McVicker, Krzysztof Kozlowski, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-clk, Krzysztof Kozlowski,
	kernel-team

Hi Peter,

On Mon, 2025-10-13 at 21:51 +0100, Peter Griffin wrote:
> Update exynos_arm64_init_clocks() so that it enables the automatic clock
> mode bits in the CMU option register if the auto_clock_gate flag and
> option_offset fields are set for the CMU.
> 
> The CMU option register bits are global and effect every clock component in
> the CMU, as such clearing the GATE_ENABLE_HWACG bit and setting GATE_MANUAL
> bit on every gate register is only required if auto_clock_gate is false.
> 
> Additionally if auto_clock_gate is enabled the dynamic root clock gating
> and memclk registers will be configured in the corresponding CMUs sysreg
> bank. These registers are exposed via syscon, so the register
> suspend/resume paths are updated to handle using a regmap.
> 
> As many gates for various Samsung SoCs are already exposed in the Samsung
> clock drivers a new samsung_auto_clk_gate_ops is implemented. This uses
> some CMU debug registers to report whether clocks are enabled or disabled
> when operating in automatic mode. This allows
> /sys/kernel/debug/clk/clk_summary to still dump the entire clock tree and
> correctly report the status of each clock in the system.
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>

It's great to see some effort on this topic as this makes the clock handling
on new Exynos much easier and less error prone and time-consuming, and having
the hardware decide which clocks need to be enabled at a given point in time
allows for a significantly more dynamic environment.

Just some initial comments below.

> ---
>  drivers/clk/samsung/clk-exynos-arm64.c   |  47 +++++++--
>  drivers/clk/samsung/clk-exynos4.c        |   6 +-
>  drivers/clk/samsung/clk-exynos4412-isp.c |   4 +-
>  drivers/clk/samsung/clk-exynos5250.c     |   2 +-
>  drivers/clk/samsung/clk-exynos5420.c     |   4 +-
>  drivers/clk/samsung/clk.c                | 161 ++++++++++++++++++++++++++++---
>  drivers/clk/samsung/clk.h                |  49 +++++++++-
>  7 files changed, 244 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/clk/samsung/clk-exynos-arm64.c b/drivers/clk/samsung/clk-exynos-arm64.c
> index bf7de21f329ec89069dcf817ca578fcf9b2d9809..c302c836e8f9f6270753d86d7d986c88e6762f4f 100644
> --- a/drivers/clk/samsung/clk-exynos-arm64.c
> +++ b/drivers/clk/samsung/clk-exynos-arm64.c
> @@ -24,6 +24,16 @@
>  #define GATE_MANUAL		BIT(20)
>  #define GATE_ENABLE_HWACG	BIT(28)
>  
> +/* Option register bits */
> +#define OPT_EN_DBG			BIT(31)
> +#define OPT_UNKNOWN			BIT(30)

Except for AOC, bit 30 is called ENABLE_LAYER2_CTRL in all gs101 CMUs.

> +#define OPT_EN_PWR_MANAGEMENT		BIT(29)
> +#define OPT_EN_AUTO_GATING		BIT(28)
> +#define OPT_EN_MEM_PM_GATING		BIT(24)
> +
> +#define CMU_OPT_GLOBAL_EN_AUTO_GATING	(OPT_EN_DBG | OPT_UNKNOWN | \
> +	OPT_EN_PWR_MANAGEMENT | OPT_EN_AUTO_GATING | OPT_EN_MEM_PM_GATING)
> +
>  /* PLL_CONx_PLL register offsets range */
>  #define PLL_CON_OFF_START	0x100
>  #define PLL_CON_OFF_END		0x600
> @@ -37,6 +47,8 @@ struct exynos_arm64_cmu_data {
>  	unsigned int nr_clk_save;
>  	const struct samsung_clk_reg_dump *clk_suspend;
>  	unsigned int nr_clk_suspend;
> +	struct samsung_clk_reg_dump *clk_sysreg_save;
> +	unsigned int nr_clk_sysreg;
>  
>  	struct clk *clk;
>  	struct clk **pclks;
> @@ -82,13 +94,28 @@ static void __init exynos_arm64_init_clocks(struct device_node *np,
>  	if (!reg_base)
>  		panic("%s: failed to map registers\n", __func__);
>  
> +	if (cmu->auto_clock_gate && cmu->option_offset) {
> +		/*
> +		 * Enable the global automatic mode for the entire CMU.
> +		 * This overrides the individual HWACG bits in each of the
> +		 * individual gate, mux and qch registers.
> +		 */
> +		writel(CMU_OPT_GLOBAL_EN_AUTO_GATING,
> +		       reg_base + cmu->option_offset);
> +	}
> +
>  	for (i = 0; i < reg_offs_len; ++i) {
>  		void __iomem *reg = reg_base + reg_offs[i];
>  		u32 val;
>  
>  		if (cmu->manual_plls && is_pll_con1_reg(reg_offs[i])) {
>  			writel(PLL_CON1_MANUAL, reg);
> -		} else if (is_gate_reg(reg_offs[i])) {
> +		} else if (is_gate_reg(reg_offs[i]) && !cmu->auto_clock_gate) {
> +			/*
> +			 * Setting GATE_MANUAL bit (which is described in TRM as
> +			 * reserved!) overrides the global CMU automatic mode
> +			 * option.
> +			 */
>  			val = readl(reg);
>  			val |= GATE_MANUAL;
>  			val &= ~GATE_ENABLE_HWACG;
> @@ -219,7 +246,7 @@ void __init exynos_arm64_register_cmu(struct device *dev,
>   * Return: 0 on success, or negative error code on error.
>   */
>  int __init exynos_arm64_register_cmu_pm(struct platform_device *pdev,
> -					bool set_manual)
> +					bool init_clk_regs)

kerneldoc needs updating

>  {
>  	const struct samsung_cmu_info *cmu;
>  	struct device *dev = &pdev->dev;
> @@ -249,7 +276,7 @@ int __init exynos_arm64_register_cmu_pm(struct platform_device *pdev,
>  		dev_err(dev, "%s: could not enable bus clock %s; err = %d\n",
>  		       __func__, cmu->clk_name, ret);
>  
> -	if (set_manual)
> +	if (init_clk_regs)
>  		exynos_arm64_init_clocks(np, cmu);
>  
>  	reg_base = devm_platform_ioremap_resource(pdev, 0);
> @@ -280,14 +307,18 @@ int exynos_arm64_cmu_suspend(struct device *dev)
>  	struct exynos_arm64_cmu_data *data = dev_get_drvdata(dev);
>  	int i;
>  
> -	samsung_clk_save(data->ctx->reg_base, data->clk_save,
> +	samsung_clk_save(data->ctx->reg_base, NULL, data->clk_save,
>  			 data->nr_clk_save);
>  
> +	if (data->ctx->sysreg)
> +		samsung_clk_save(NULL, data->ctx->sysreg, data->clk_save,
> +				 data->nr_clk_save);
> +

I think this should be

+		samsung_clk_save(NULL, data->ctx->sysreg, data->clk_sysreg_save,
+				 data->nr_clk_sysreg);

?

>  	for (i = 0; i < data->nr_pclks; i++)
>  		clk_prepare_enable(data->pclks[i]);
>  
>  	/* For suspend some registers have to be set to certain values */
> -	samsung_clk_restore(data->ctx->reg_base, data->clk_suspend,
> +	samsung_clk_restore(data->ctx->reg_base, NULL, data->clk_suspend,
>  			    data->nr_clk_suspend);
>  
>  	for (i = 0; i < data->nr_pclks; i++)
> @@ -308,9 +339,13 @@ int exynos_arm64_cmu_resume(struct device *dev)
>  	for (i = 0; i < data->nr_pclks; i++)
>  		clk_prepare_enable(data->pclks[i]);
>  
> -	samsung_clk_restore(data->ctx->reg_base, data->clk_save,
> +	samsung_clk_restore(data->ctx->reg_base, NULL, data->clk_save,
>  			    data->nr_clk_save);
>  
> +	if (data->ctx->sysreg)
> +		samsung_clk_restore(NULL, data->ctx->sysreg, data->clk_save,
> +				    data->nr_clk_save);
> +

dito.

>  	for (i = 0; i < data->nr_pclks; i++)
>  		clk_disable_unprepare(data->pclks[i]);
> 

[...]

> diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c
> index dbc9925ca8f46e951dfb5d391c0e744ca370abcc..07b2948ae7ea48f126ab420be57d8c2705979464 100644
> --- a/drivers/clk/samsung/clk.c
> +++ b/drivers/clk/samsung/clk.c
> @@ -12,8 +12,10 @@
>  #include <linux/clkdev.h>
>  #include <linux/clk-provider.h>
>  #include <linux/io.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/of_address.h>
> +#include <linux/regmap.h>
>  #include <linux/syscore_ops.h>
>  
>  #include "clk.h"
> @@ -21,19 +23,29 @@
>  static LIST_HEAD(clock_reg_cache_list);
>  
>  void samsung_clk_save(void __iomem *base,
> +				    struct regmap *regmap,
>  				    struct samsung_clk_reg_dump *rd,
>  				    unsigned int num_regs)
>  {
> -	for (; num_regs > 0; --num_regs, ++rd)
> -		rd->value = readl(base + rd->offset);
> +	for (; num_regs > 0; --num_regs, ++rd) {
> +		if (base)
> +			rd->value = readl(base + rd->offset);
> +		if (regmap)

Should this be else if?

> +			regmap_read(regmap, rd->offset, &rd->value);

Otherwise users that (incorrectly) pass both base and regmap would
cause this to end up reading the offset from the wrong memory
region.

At least this way that won't happen, but maybe this constrains should
be made even more explicit.

> +	}
>  }
>  
>  void samsung_clk_restore(void __iomem *base,
> +				      struct regmap *regmap,
>  				      const struct samsung_clk_reg_dump *rd,
>  				      unsigned int num_regs)
>  {
> -	for (; num_regs > 0; --num_regs, ++rd)
> -		writel(rd->value, base + rd->offset);
> +	for (; num_regs > 0; --num_regs, ++rd) {
> +		if (base)
> +			writel(rd->value, base + rd->offset);
> +		if (regmap)
> +			regmap_write(regmap, rd->offset, rd->value);

dito.

> +	}
>  }
>  
>  struct samsung_clk_reg_dump *samsung_clk_alloc_reg_dump(
> @@ -227,6 +239,82 @@ void __init samsung_clk_register_div(struct samsung_clk_provider *ctx,
>  	}
>  }
>  
> +#define ACG_MSK GENMASK(6, 4)
> +#define CLK_IDLE GENMASK(5, 4)
> +static int samsung_auto_clk_gate_is_en(struct clk_hw *hw)
> +{
> +	u32 reg;
> +	struct clk_gate *gate = to_clk_gate(hw);
> +
> +	reg = readl(gate->reg);
> +	return ((reg & ACG_MSK) == CLK_IDLE) ? 0 : 1;
> +}
> +
> +/* enable and disable are nops in automatic clock mode */
> +static int samsung_auto_clk_gate_en(struct clk_hw *hw)
> +{
> +	return 0;
> +}
> +
> +static void samsung_auto_clk_gate_dis(struct clk_hw *hw)
> +{
> +}
> +
> +static const struct clk_ops samsung_auto_clk_gate_ops = {
> +	.enable = samsung_auto_clk_gate_en,
> +	.disable = samsung_auto_clk_gate_dis,
> +	.is_enabled = samsung_auto_clk_gate_is_en,
> +};
> +
> +struct clk_hw *samsung_register_auto_gate(struct device *dev,
> +		struct device_node *np, const char *name,
> +		const char *parent_name, const struct clk_hw *parent_hw,
> +		const struct clk_parent_data *parent_data,
> +		unsigned long flags,
> +		void __iomem *reg, u8 bit_idx,
> +		u8 clk_gate_flags, spinlock_t *lock)
> +{
> +	struct clk_gate *gate;
> +	struct clk_hw *hw;
> +	struct clk_init_data init = {};
> +	int ret = -EINVAL;
> +
> +	/* allocate the gate */
> +	gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> +	if (!gate)
> +		return ERR_PTR(-ENOMEM);
> +
> +	init.name = name;
> +	init.ops = &samsung_auto_clk_gate_ops;
> +	init.flags = flags;
> +	init.parent_names = parent_name ? &parent_name : NULL;
> +	init.parent_hws = parent_hw ? &parent_hw : NULL;
> +	init.parent_data = parent_data;
> +	if (parent_name || parent_hw || parent_data)
> +		init.num_parents = 1;
> +	else
> +		init.num_parents = 0;
> +
> +	/* struct clk_gate assignments */
> +	gate->reg = reg;
> +	gate->bit_idx = bit_idx;
> +	gate->flags = clk_gate_flags;
> +	gate->lock = lock;
> +	gate->hw.init = &init;
> +
> +	hw = &gate->hw;
> +	if (dev || !np)
> +		ret = clk_hw_register(dev, hw);
> +	else if (np)
> +		ret = of_clk_hw_register(np, hw);
> +	if (ret) {
> +		kfree(gate);
> +		hw = ERR_PTR(ret);
> +	}
> +
> +	return hw;
> +}
> +
>  /* register a list of gate clocks */
>  void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx,
>  				const struct samsung_gate_clock *list,
> @@ -234,11 +322,21 @@ void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx,
>  {
>  	struct clk_hw *clk_hw;
>  	unsigned int idx;
> +	void __iomem *reg_offs;
>  
>  	for (idx = 0; idx < nr_clk; idx++, list++) {
> -		clk_hw = clk_hw_register_gate(ctx->dev, list->name, list->parent_name,
> -				list->flags, ctx->reg_base + list->offset,
> +		reg_offs = ctx->reg_base + list->offset;
> +
> +		if (ctx->auto_clock_gate && ctx->gate_dbg_offset)
> +			clk_hw = samsung_register_auto_gate(ctx->dev, NULL,
> +				list->name, list->parent_name, NULL, NULL,
> +				list->flags, reg_offs + ctx->gate_dbg_offset,
>  				list->bit_idx, list->gate_flags, &ctx->lock);
> +		else
> +			clk_hw = clk_hw_register_gate(ctx->dev, list->name,
> +				list->parent_name, list->flags,
> +				ctx->reg_base + list->offset, list->bit_idx,
> +				list->gate_flags, &ctx->lock);
>  		if (IS_ERR(clk_hw)) {
>  			pr_err("%s: failed to register clock %s\n", __func__,
>  				list->name);

Maybe the error message should include the actual error clk_hw here?

> @@ -276,10 +374,11 @@ static int samsung_clk_suspend(void)
>  	struct samsung_clock_reg_cache *reg_cache;
>  
>  	list_for_each_entry(reg_cache, &clock_reg_cache_list, node) {
> -		samsung_clk_save(reg_cache->reg_base, reg_cache->rdump,
> -				reg_cache->rd_num);
> -		samsung_clk_restore(reg_cache->reg_base, reg_cache->rsuspend,
> -				reg_cache->rsuspend_num);
> +		samsung_clk_save(reg_cache->reg_base, reg_cache->sysreg,
> +				 reg_cache->rdump, reg_cache->rd_num);
> +		samsung_clk_restore(reg_cache->reg_base, reg_cache->sysreg,
> +				    reg_cache->rsuspend,
> +				    reg_cache->rsuspend_num);
>  	}
>  	return 0;
>  }
> @@ -289,8 +388,8 @@ static void samsung_clk_resume(void)
>  	struct samsung_clock_reg_cache *reg_cache;
>  
>  	list_for_each_entry(reg_cache, &clock_reg_cache_list, node)
> -		samsung_clk_restore(reg_cache->reg_base, reg_cache->rdump,
> -				reg_cache->rd_num);
> +		samsung_clk_restore(reg_cache->reg_base, reg_cache->sysreg,
> +				    reg_cache->rdump, reg_cache->rd_num);
>  }
>  
>  static struct syscore_ops samsung_clk_syscore_ops = {
> @@ -299,6 +398,7 @@ static struct syscore_ops samsung_clk_syscore_ops = {
>  };
>  
>  void samsung_clk_extended_sleep_init(void __iomem *reg_base,
> +			struct regmap *sysreg,
>  			const unsigned long *rdump,
>  			unsigned long nr_rdump,
>  			const struct samsung_clk_reg_dump *rsuspend,
> @@ -319,6 +419,7 @@ void samsung_clk_extended_sleep_init(void __iomem *reg_base,
>  		register_syscore_ops(&samsung_clk_syscore_ops);
>  
>  	reg_cache->reg_base = reg_base;
> +	reg_cache->sysreg = sysreg;
>  	reg_cache->rd_num = nr_rdump;
>  	reg_cache->rsuspend = rsuspend;
>  	reg_cache->rsuspend_num = nr_rsuspend;
> @@ -334,6 +435,12 @@ void samsung_clk_extended_sleep_init(void __iomem *reg_base,
>  void __init samsung_cmu_register_clocks(struct samsung_clk_provider *ctx,
>  					const struct samsung_cmu_info *cmu)
>  {
> +	ctx->auto_clock_gate = cmu->auto_clock_gate;
> +	ctx->gate_dbg_offset = cmu->gate_dbg_offset;
> +	ctx->option_offset = cmu->option_offset;
> +	ctx->drcg_offset = cmu->drcg_offset;
> +	ctx->memclk_offset = cmu->memclk_offset;
> +
>  	if (cmu->pll_clks)
>  		samsung_clk_register_pll(ctx, cmu->pll_clks, cmu->nr_pll_clks);
>  	if (cmu->mux_clks)
> @@ -353,6 +460,31 @@ void __init samsung_cmu_register_clocks(struct samsung_clk_provider *ctx,
>  		samsung_clk_register_cpu(ctx, cmu->cpu_clks, cmu->nr_cpu_clks);
>  }
>  
> +/* Enable Dynamic Root Clock Gating of bus components*/

missing space before */

> +void samsung_en_dyn_root_clk_gating(struct device_node *np,
> +				    struct samsung_clk_provider *ctx,
> +				    const struct samsung_cmu_info *cmu)
> +{
> +	if (ctx && !ctx->auto_clock_gate)

ctx can not be NULL here. If it could, then the following lines would also
need updating, but again, it can not be NULL, if ctx allocation fails, the
code will panic() before this here is reached.

> +		return;
> +
> +	ctx->sysreg = syscon_regmap_lookup_by_phandle(np, "samsung,sysreg");
> +	if (!IS_ERR_OR_NULL(ctx->sysreg)) {
> +		regmap_write(ctx->sysreg, ctx->drcg_offset, 0xffffffff);

What does 0xffffffff mean? Maybe a macro or at least a comment would help.

> +		/* not every sysreg controller has memclk reg*/

missing space before */

Can you please check all such comments, since it seems to be recurring :-)

+		if (ctx->memclk_offset)
> +			regmap_write_bits(ctx->sysreg, ctx->memclk_offset, 0x1, 0x0);
> +
> +		samsung_clk_extended_sleep_init(NULL, ctx->sysreg,
> +						cmu->sysreg_clk_regs,
> +						cmu->nr_sysreg_clk_regs,
> +						NULL, 0);
> +	} else {
> +		pr_warn("%pOF: Unable to get CMU sysreg\n", np);
> +		ctx->sysreg = NULL;
> +	}
> +}
> +
>  /*
>   * Common function which registers plls, muxes, dividers and gates
>   * for each CMU. It also add CMU register list to register cache.
> @@ -374,11 +506,14 @@ struct samsung_clk_provider * __init samsung_cmu_register_one(
>  	samsung_cmu_register_clocks(ctx, cmu);
>  
>  	if (cmu->clk_regs)
> -		samsung_clk_extended_sleep_init(reg_base,
> +		samsung_clk_extended_sleep_init(reg_base, NULL,
>  			cmu->clk_regs, cmu->nr_clk_regs,
>  			cmu->suspend_regs, cmu->nr_suspend_regs);
>  
>  	samsung_clk_of_add_provider(np, ctx);
>  
> +	/* sysreg DT nodes reference a clock in this CMU */
> +	samsung_en_dyn_root_clk_gating(np, ctx, cmu);
> +

samsung_cmu_register_one() is not called when the clocks are registered
using exynos_arm64_register_cmu_pm(). Does the pm version need a call to
samsung_en_dyn_root_clk_gating() somewhere?

>  	return ctx;
>  }
> diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
> index 18660c1ac6f0106b17b9efc9c6b3cd62d46f7b82..b719e057f45489e9d92ba54031fe633a8c9264ce 100644
> --- a/drivers/clk/samsung/clk.h
> +++ b/drivers/clk/samsung/clk.h
> @@ -12,6 +12,7 @@
>  
>  #include <linux/clk-provider.h>
>  #include <linux/mod_devicetable.h>
> +#include <linux/regmap.h>
>  #include "clk-pll.h"
>  #include "clk-cpu.h"
>  
> @@ -19,13 +20,25 @@
>   * struct samsung_clk_provider - information about clock provider
>   * @reg_base: virtual address for the register base
>   * @dev: clock provider device needed for runtime PM
> + * @sysreg: syscon regmap for clock-provider sysreg controller
>   * @lock: maintains exclusion between callbacks for a given clock-provider
> + * @auto_clock_gate: enable auto clk mode for all clocks in clock-provider
> + * @gate_dbg_offset: gate debug reg offset. Used for all gates in auto clk mode
> + * @option_offset: option reg offset. Enables auto mode for clock-provider
> + * @drcg_offset: dynamic root clk gate enable register offset
> + * @memclk_offset: memclk enable register offset

The last two are in the sysreg space, maybe the comment should mention that.

>   * @clk_data: holds clock related data like clk_hw* and number of clocks
>   */
>  struct samsung_clk_provider {
>  	void __iomem *reg_base;
>  	struct device *dev;
> +	struct regmap *sysreg;
>  	spinlock_t lock;
> +	bool auto_clock_gate;
> +	u32 gate_dbg_offset;
> +	u32 option_offset;
> +	u32 drcg_offset;
> +	u32 memclk_offset;
>  	/* clk_data must be the last entry due to variable length 'hws' array */
>  	struct clk_hw_onecell_data clk_data;
>  };
> @@ -310,6 +323,7 @@ struct samsung_cpu_clock {
>  struct samsung_clock_reg_cache {
>  	struct list_head node;
>  	void __iomem *reg_base;
> +	struct regmap *sysreg;
>  	struct samsung_clk_reg_dump *rdump;
>  	unsigned int rd_num;
>  	const struct samsung_clk_reg_dump *rsuspend;
> @@ -338,7 +352,14 @@ struct samsung_clock_reg_cache {
>   * @suspend_regs: list of clock registers to set before suspend
>   * @nr_suspend_regs: count of clock registers in @suspend_regs
>   * @clk_name: name of the parent clock needed for CMU register access
> + * @sysreg_clk_regs: list of sysreg clock registers
> + * @nr_sysreg_clk_regs: count of clock registers in @sysreg_clk_regs
>   * @manual_plls: Enable manual control for PLL clocks
> + * @auto_clock_gate: enable auto clock mode for all components in CMU
> + * @gate_dbg_offset: gate debug reg offset. Used by all gates in auto clk mode
> + * @option_offset: option reg offset. Enables auto clk mode for entire CMU
> + * @drcg_offset: dynamic root clk gate enable register offset
> + * @memclk_offset: memclk enable register offset

dito.

Cheers,
Andre'


>   */
>  struct samsung_cmu_info {
>  	const struct samsung_pll_clock *pll_clks;
> @@ -364,8 +385,16 @@ struct samsung_cmu_info {
>  	unsigned int nr_suspend_regs;
>  	const char *clk_name;
>  
> +	const unsigned long *sysreg_clk_regs;
> +	unsigned int nr_sysreg_clk_regs;
> +
>  	/* ARM64 Exynos CMUs */
>  	bool manual_plls;
> +	bool auto_clock_gate;
> +	u32 gate_dbg_offset;
> +	u32 option_offset;
> +	u32 drcg_offset;
> +	u32 memclk_offset;
>  };
>  
>  struct samsung_clk_provider *samsung_clk_init(struct device *dev,
> @@ -415,6 +444,7 @@ struct samsung_clk_provider *samsung_cmu_register_one(
>  
>  #ifdef CONFIG_PM_SLEEP
>  void samsung_clk_extended_sleep_init(void __iomem *reg_base,
> +			struct regmap *sysreg,
>  			const unsigned long *rdump,
>  			unsigned long nr_rdump,
>  			const struct samsung_clk_reg_dump *rsuspend,
> @@ -426,17 +456,32 @@ static inline void samsung_clk_extended_sleep_init(void __iomem *reg_base,
>  			const struct samsung_clk_reg_dump *rsuspend,
>  			unsigned long nr_rsuspend) {}
>  #endif
> -#define samsung_clk_sleep_init(reg_base, rdump, nr_rdump) \
> -	samsung_clk_extended_sleep_init(reg_base, rdump, nr_rdump, NULL, 0)
> +#define samsung_clk_sleep_init(reg_base, sysreg, rdump, nr_rdump)	   \
> +	samsung_clk_extended_sleep_init(reg_base, sysreg, rdump, nr_rdump, \
> +					NULL, 0)
>  
>  void samsung_clk_save(void __iomem *base,
> +			struct regmap *regmap,
>  			struct samsung_clk_reg_dump *rd,
>  			unsigned int num_regs);
>  void samsung_clk_restore(void __iomem *base,
> +			struct regmap *regmap,
>  			const struct samsung_clk_reg_dump *rd,
>  			unsigned int num_regs);
>  struct samsung_clk_reg_dump *samsung_clk_alloc_reg_dump(
>  			const unsigned long *rdump,
>  			unsigned long nr_rdump);
>  
> +void samsung_en_dyn_root_clk_gating(struct device_node *np,
> +				struct samsung_clk_provider *ctx,
> +				const struct samsung_cmu_info *cmu);
> +
> +struct clk_hw *samsung_register_auto_gate(struct device *dev,
> +		struct device_node *np, const char *name,
> +		const char *parent_name, const struct clk_hw *parent_hw,
> +		const struct clk_parent_data *parent_data,
> +		unsigned long flags,
> +		void __iomem *reg, u8 bit_idx,
> +		u8 clk_gate_flags, spinlock_t *lock);
> +
>  #endif /* __SAMSUNG_CLK_H */

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

* Re: [PATCH 2/9] dt-bindings: clock: google,gs101-clock: add samsung,sysreg property as required
  2025-10-13 20:51 ` [PATCH 2/9] dt-bindings: clock: google,gs101-clock: add samsung,sysreg property as required Peter Griffin
  2025-10-15 18:32   ` Rob Herring (Arm)
  2025-10-16  5:53   ` André Draszik
@ 2025-10-21 19:22   ` Krzysztof Kozlowski
  2 siblings, 0 replies; 30+ messages in thread
From: Krzysztof Kozlowski @ 2025-10-21 19:22 UTC (permalink / raw)
  To: Peter Griffin, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Alim Akhtar, André Draszik, Tudor Ambarus, Michael Turquette,
	Stephen Boyd, Sam Protsenko, Sylwester Nawrocki, Chanwoo Choi
  Cc: Will McVicker, devicetree, linux-arm-kernel, linux-samsung-soc,
	linux-kernel, linux-clk, Krzysztof Kozlowski, kernel-team

On 13/10/2025 22:51, Peter Griffin wrote:
> Update the bindings documentation so that all CMUs (with the exception of
> gs101-cmu-top) have samsung,sysreg as a required property.


Why? You described the patch contents, which I can see. I don't
understand why we need it, especially that this is ABI break.

> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  .../bindings/clock/google,gs101-clock.yaml         | 23 +++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml b/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml
> index caf442ead24bda57e531420d8a7d8de8713032ae..5cfe98d9ba895d5207fffc82f3fd55b602b4a2bb 100644
> --- a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml
> +++ b/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml
> @@ -49,6 +49,11 @@ properties:
>    reg:
>      maxItems: 1
>  
> +  samsung,sysreg:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      Phandle to system registers interface.


Your description should say here: for what purpose, how this device is
going to use it.

> +
>  required:
>    - compatible
>    - "#clock-cells"
> @@ -163,6 +168,22 @@ allOf:
>              - const: bus
>              - const: ip
Best regards,
Krzysztof

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

* Re: (subset) [PATCH 1/9] dt-bindings: soc: samsung: exynos-sysreg: add gs101 hsi0 and misc compatibles
  2025-10-13 20:51 ` [PATCH 1/9] dt-bindings: soc: samsung: exynos-sysreg: add gs101 hsi0 and misc compatibles Peter Griffin
  2025-10-15 18:31   ` Rob Herring (Arm)
  2025-10-16  5:44   ` André Draszik
@ 2025-10-21 19:27   ` Krzysztof Kozlowski
  2 siblings, 0 replies; 30+ messages in thread
From: Krzysztof Kozlowski @ 2025-10-21 19:27 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar,
	André Draszik, Tudor Ambarus, Michael Turquette,
	Stephen Boyd, Sam Protsenko, Sylwester Nawrocki, Chanwoo Choi,
	Peter Griffin
  Cc: Will McVicker, Krzysztof Kozlowski, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-clk, kernel-team


On Mon, 13 Oct 2025 21:51:30 +0100, Peter Griffin wrote:
> Add dedicated compatibles for gs101 hsi0 and misc sysreg controllers to the
> documentation.
> 
> 

Applied, thanks!

[1/9] dt-bindings: soc: samsung: exynos-sysreg: add gs101 hsi0 and misc compatibles
      https://git.kernel.org/krzk/linux/c/33fd5a7103959113ea3b60389a7582ec0cc2f15e

Best regards,
-- 
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


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

* Re: (subset) [PATCH 3/9] arm64: dts: exynos: gs101: add sysreg_misc and sysreg_hsi0 nodes
  2025-10-13 20:51 ` [PATCH 3/9] arm64: dts: exynos: gs101: add sysreg_misc and sysreg_hsi0 nodes Peter Griffin
  2025-10-16  5:56   ` André Draszik
@ 2025-10-21 19:27   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 30+ messages in thread
From: Krzysztof Kozlowski @ 2025-10-21 19:27 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar,
	André Draszik, Tudor Ambarus, Michael Turquette,
	Stephen Boyd, Sam Protsenko, Sylwester Nawrocki, Chanwoo Choi,
	Peter Griffin
  Cc: Will McVicker, Krzysztof Kozlowski, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-clk, kernel-team


On Mon, 13 Oct 2025 21:51:32 +0100, Peter Griffin wrote:
> Add syscon DT node for the hsi0 and misc sysreg controllers. These will be
> referenced by their respective CMU nodes in future patchs.
> 
> 

Applied, thanks!

[3/9] arm64: dts: exynos: gs101: add sysreg_misc and sysreg_hsi0 nodes
      https://git.kernel.org/krzk/linux/c/08d9d0d9ae6f9f83bc1802a3f8ab2a4801ccd3e6

Best regards,
-- 
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


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

* Re: (subset) [PATCH 4/9] arm64: dts: exynos: gs101: fix clock module unit reg sizes
  2025-10-13 20:51 ` [PATCH 4/9] arm64: dts: exynos: gs101: fix clock module unit reg sizes Peter Griffin
  2025-10-16  8:22   ` André Draszik
@ 2025-10-21 19:27   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 30+ messages in thread
From: Krzysztof Kozlowski @ 2025-10-21 19:27 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar,
	André Draszik, Tudor Ambarus, Michael Turquette,
	Stephen Boyd, Sam Protsenko, Sylwester Nawrocki, Chanwoo Choi,
	Peter Griffin
  Cc: Will McVicker, Krzysztof Kozlowski, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-clk, kernel-team


On Mon, 13 Oct 2025 21:51:33 +0100, Peter Griffin wrote:
> The memory map lists each clock module unit as having a size of
> 0x10000. Additionally there are some undocumented registers in this region
> that need to be used for automatic clock gating mode. Some of those
> registers also need to be saved/restored on suspend & resume.
> 
> 

Applied, thanks!

[4/9] arm64: dts: exynos: gs101: fix clock module unit reg sizes
      https://git.kernel.org/krzk/linux/c/ddb2a16804d005a96e8b5ffc0925e2f5bff65767

Best regards,
-- 
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


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

* Re: (subset) [PATCH 5/9] arm64: dts: exynos: gs101: fix sysreg_apm reg property
  2025-10-13 20:51 ` [PATCH 5/9] arm64: dts: exynos: gs101: fix sysreg_apm reg property Peter Griffin
  2025-10-16  8:23   ` André Draszik
@ 2025-10-21 19:27   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 30+ messages in thread
From: Krzysztof Kozlowski @ 2025-10-21 19:27 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar,
	André Draszik, Tudor Ambarus, Michael Turquette,
	Stephen Boyd, Sam Protsenko, Sylwester Nawrocki, Chanwoo Choi,
	Peter Griffin
  Cc: Will McVicker, Krzysztof Kozlowski, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-clk, kernel-team


On Mon, 13 Oct 2025 21:51:34 +0100, Peter Griffin wrote:
> Both the start address and size are incorrect for the apm_sysreg DT
> node. Update to match the TRM (rather than how it was defined downstream).
> 
> 

Applied, thanks!

[5/9] arm64: dts: exynos: gs101: fix sysreg_apm reg property
      https://git.kernel.org/krzk/linux/c/4348c22a4f15dbef1314f1a353d7f053b24e9ace

Best regards,
-- 
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


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

* Re: [PATCH 7/9] clk: samsung: Implement automatic clock gating mode for CMUs
  2025-10-13 20:51 ` [PATCH 7/9] clk: samsung: Implement automatic clock gating mode for CMUs Peter Griffin
  2025-10-14 14:46   ` kernel test robot
  2025-10-16  9:40   ` André Draszik
@ 2025-10-21 19:45   ` Krzysztof Kozlowski
  2025-10-28  9:29     ` Peter Griffin
  2 siblings, 1 reply; 30+ messages in thread
From: Krzysztof Kozlowski @ 2025-10-21 19:45 UTC (permalink / raw)
  To: Peter Griffin, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Alim Akhtar, André Draszik, Tudor Ambarus, Michael Turquette,
	Stephen Boyd, Sam Protsenko, Sylwester Nawrocki, Chanwoo Choi
  Cc: Will McVicker, devicetree, linux-arm-kernel, linux-samsung-soc,
	linux-kernel, linux-clk, Krzysztof Kozlowski, kernel-team

On 13/10/2025 22:51, Peter Griffin wrote:
> Update exynos_arm64_init_clocks() so that it enables the automatic clock
> mode bits in the CMU option register if the auto_clock_gate flag and
> option_offset fields are set for the CMU.
> 
> The CMU option register bits are global and effect every clock component in
> the CMU, as such clearing the GATE_ENABLE_HWACG bit and setting GATE_MANUAL
> bit on every gate register is only required if auto_clock_gate is false.
> 
> Additionally if auto_clock_gate is enabled the dynamic root clock gating
> and memclk registers will be configured in the corresponding CMUs sysreg
> bank. These registers are exposed via syscon, so the register
> suspend/resume paths are updated to handle using a regmap.
> 
> As many gates for various Samsung SoCs are already exposed in the Samsung
> clock drivers a new samsung_auto_clk_gate_ops is implemented. This uses
> some CMU debug registers to report whether clocks are enabled or disabled
> when operating in automatic mode. This allows
> /sys/kernel/debug/clk/clk_summary to still dump the entire clock tree and
> correctly report the status of each clock in the system.
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  drivers/clk/samsung/clk-exynos-arm64.c   |  47 +++++++--
>  drivers/clk/samsung/clk-exynos4.c        |   6 +-
>  drivers/clk/samsung/clk-exynos4412-isp.c |   4 +-
>  drivers/clk/samsung/clk-exynos5250.c     |   2 +-
>  drivers/clk/samsung/clk-exynos5420.c     |   4 +-
>  drivers/clk/samsung/clk.c                | 161 ++++++++++++++++++++++++++++---
>  drivers/clk/samsung/clk.h                |  49 +++++++++-
>  7 files changed, 244 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/clk/samsung/clk-exynos-arm64.c b/drivers/clk/samsung/clk-exynos-arm64.c
> index bf7de21f329ec89069dcf817ca578fcf9b2d9809..c302c836e8f9f6270753d86d7d986c88e6762f4f 100644
> --- a/drivers/clk/samsung/clk-exynos-arm64.c
> +++ b/drivers/clk/samsung/clk-exynos-arm64.c
> @@ -24,6 +24,16 @@
>  #define GATE_MANUAL		BIT(20)
>  #define GATE_ENABLE_HWACG	BIT(28)
>  
> +/* Option register bits */
> +#define OPT_EN_DBG			BIT(31)
> +#define OPT_UNKNOWN			BIT(30)
> +#define OPT_EN_PWR_MANAGEMENT		BIT(29)
> +#define OPT_EN_AUTO_GATING		BIT(28)
> +#define OPT_EN_MEM_PM_GATING		BIT(24)

Please reverse, from lowest to highest number (LSB -> MSB).

> +
> +#define CMU_OPT_GLOBAL_EN_AUTO_GATING	(OPT_EN_DBG | OPT_UNKNOWN | \
> +	OPT_EN_PWR_MANAGEMENT | OPT_EN_AUTO_GATING | OPT_EN_MEM_PM_GATING)
> +
>  /* PLL_CONx_PLL register offsets range */
>  #define PLL_CON_OFF_START	0x100
>  #define PLL_CON_OFF_END		0x600
> @@ -37,6 +47,8 @@ struct exynos_arm64_cmu_data {
>  	unsigned int nr_clk_save;
>  	const struct samsung_clk_reg_dump *clk_suspend;
>  	unsigned int nr_clk_suspend;
> +	struct samsung_clk_reg_dump *clk_sysreg_save;
> +	unsigned int nr_clk_sysreg;
>  
>  	struct clk *clk;
>  	struct clk **pclks;
> @@ -82,13 +94,28 @@ static void __init exynos_arm64_init_clocks(struct device_node *np,
>  	if (!reg_base)
>  		panic("%s: failed to map registers\n", __func__);
>  
> +	if (cmu->auto_clock_gate && cmu->option_offset) {
> +		/*
> +		 * Enable the global automatic mode for the entire CMU.
> +		 * This overrides the individual HWACG bits in each of the
> +		 * individual gate, mux and qch registers.
> +		 */
> +		writel(CMU_OPT_GLOBAL_EN_AUTO_GATING,
> +		       reg_base + cmu->option_offset);
> +	}
> +
>  	for (i = 0; i < reg_offs_len; ++i) {
>  		void __iomem *reg = reg_base + reg_offs[i];
>  		u32 val;
>  
>  		if (cmu->manual_plls && is_pll_con1_reg(reg_offs[i])) {
>  			writel(PLL_CON1_MANUAL, reg);
> -		} else if (is_gate_reg(reg_offs[i])) {
> +		} else if (is_gate_reg(reg_offs[i]) && !cmu->auto_clock_gate) {
> +			/*
> +			 * Setting GATE_MANUAL bit (which is described in TRM as
> +			 * reserved!) overrides the global CMU automatic mode
> +			 * option.
> +			 */
>  			val = readl(reg);
>  			val |= GATE_MANUAL;
>  			val &= ~GATE_ENABLE_HWACG;
> @@ -219,7 +246,7 @@ void __init exynos_arm64_register_cmu(struct device *dev,
>   * Return: 0 on success, or negative error code on error.
>   */
>  int __init exynos_arm64_register_cmu_pm(struct platform_device *pdev,
> -					bool set_manual)
> +					bool init_clk_regs)
>  {
>  	const struct samsung_cmu_info *cmu;
>  	struct device *dev = &pdev->dev;
> @@ -249,7 +276,7 @@ int __init exynos_arm64_register_cmu_pm(struct platform_device *pdev,
>  		dev_err(dev, "%s: could not enable bus clock %s; err = %d\n",
>  		       __func__, cmu->clk_name, ret);
>  
> -	if (set_manual)
> +	if (init_clk_regs)
>  		exynos_arm64_init_clocks(np, cmu);
>  
>  	reg_base = devm_platform_ioremap_resource(pdev, 0);
> @@ -280,14 +307,18 @@ int exynos_arm64_cmu_suspend(struct device *dev)
>  	struct exynos_arm64_cmu_data *data = dev_get_drvdata(dev);
>  	int i;
>  
> -	samsung_clk_save(data->ctx->reg_base, data->clk_save,
> +	samsung_clk_save(data->ctx->reg_base, NULL, data->clk_save,
>  			 data->nr_clk_save);
>  
> +	if (data->ctx->sysreg)

No need for if() here and in samsung_clk_save.

I find it confusing. You do samsung_clk_save() for regular CMU MMIO
address and then immediately - for GS101 - do the same for sysreg:

> +		samsung_clk_save(NULL, data->ctx->sysreg, data->clk_save,
> +				 data->nr_clk_save);

So this feels like you could make one call of samsung_clk_save() with
both arguments - reg_base and sysreg. It also leads to confusion - first
read from MMIO will be overwritten by sysreg read.


> +
>  	for (i = 0; i < data->nr_pclks; i++)
>  		clk_prepare_enable(data->pclks[i]);
>  
>  	/* For suspend some registers have to be set to certain values */
> -	samsung_clk_restore(data->ctx->reg_base, data->clk_suspend,
> +	samsung_clk_restore(data->ctx->reg_base, NULL, data->clk_suspend,
>  			    data->nr_clk_suspend);
>  
>  	for (i = 0; i < data->nr_pclks; i++)
> @@ -308,9 +339,13 @@ int exynos_arm64_cmu_resume(struct device *dev)
>  	for (i = 0; i < data->nr_pclks; i++)
>  		clk_prepare_enable(data->pclks[i]);
>  
> -	samsung_clk_restore(data->ctx->reg_base, data->clk_save,
> +	samsung_clk_restore(data->ctx->reg_base, NULL, data->clk_save,
>  			    data->nr_clk_save);
>  
> +	if (data->ctx->sysreg)
> +		samsung_clk_restore(NULL, data->ctx->sysreg, data->clk_save,
> +				    data->nr_clk_save);
> +
>  	for (i = 0; i < data->nr_pclks; i++)
>  		clk_disable_unprepare(data->pclks[i]);
>  
> diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
> index cc5c1644c41c08b27bc48d809a08cd8a006cbe8f..26ac9734722d1e7ed8ec3f1c0a956f26e32b92d4 100644
> --- a/drivers/clk/samsung/clk-exynos4.c
> +++ b/drivers/clk/samsung/clk-exynos4.c
> @@ -1378,15 +1378,15 @@ static void __init exynos4_clk_init(struct device_node *np,
>  	if (soc == EXYNOS4212 || soc == EXYNOS4412)
>  		exynos4x12_core_down_clock();
>  
> -	samsung_clk_extended_sleep_init(reg_base,
> +	samsung_clk_extended_sleep_init(reg_base, NULL,
>  			exynos4_clk_regs, ARRAY_SIZE(exynos4_clk_regs),
>  			src_mask_suspend, ARRAY_SIZE(src_mask_suspend));
>  	if (exynos4_soc == EXYNOS4210)
> -		samsung_clk_extended_sleep_init(reg_base,
> +		samsung_clk_extended_sleep_init(reg_base, NULL,
>  		    exynos4210_clk_save, ARRAY_SIZE(exynos4210_clk_save),
>  		    src_mask_suspend_e4210, ARRAY_SIZE(src_mask_suspend_e4210));
>  	else
> -		samsung_clk_sleep_init(reg_base, exynos4x12_clk_save,
> +		samsung_clk_sleep_init(reg_base, NULL, exynos4x12_clk_save,
>  				       ARRAY_SIZE(exynos4x12_clk_save));
>  
>  	samsung_clk_of_add_provider(np, ctx);
> diff --git a/drivers/clk/samsung/clk-exynos4412-isp.c b/drivers/clk/samsung/clk-exynos4412-isp.c
> index fa915057e109e0008ebe0b1b5d1652fd5804e82b..772bc18a1e686f23b11bf160b803becff6279637 100644
> --- a/drivers/clk/samsung/clk-exynos4412-isp.c
> +++ b/drivers/clk/samsung/clk-exynos4412-isp.c
> @@ -94,7 +94,7 @@ static int __maybe_unused exynos4x12_isp_clk_suspend(struct device *dev)
>  {
>  	struct samsung_clk_provider *ctx = dev_get_drvdata(dev);
>  
> -	samsung_clk_save(ctx->reg_base, exynos4x12_save_isp,
> +	samsung_clk_save(ctx->reg_base, NULL, exynos4x12_save_isp,
>  			 ARRAY_SIZE(exynos4x12_clk_isp_save));
>  	return 0;
>  }
> @@ -103,7 +103,7 @@ static int __maybe_unused exynos4x12_isp_clk_resume(struct device *dev)
>  {
>  	struct samsung_clk_provider *ctx = dev_get_drvdata(dev);
>  
> -	samsung_clk_restore(ctx->reg_base, exynos4x12_save_isp,
> +	samsung_clk_restore(ctx->reg_base, NULL, exynos4x12_save_isp,
>  			    ARRAY_SIZE(exynos4x12_clk_isp_save));
>  	return 0;
>  }
> diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c
> index e90d3a0848cbc24b2709c10795f6affcda404567..f97f30b29be7317db8186bac39cf52e1893eb106 100644
> --- a/drivers/clk/samsung/clk-exynos5250.c
> +++ b/drivers/clk/samsung/clk-exynos5250.c
> @@ -854,7 +854,7 @@ static void __init exynos5250_clk_init(struct device_node *np)
>  		PWR_CTRL2_CORE2_UP_RATIO | PWR_CTRL2_CORE1_UP_RATIO);
>  	__raw_writel(tmp, reg_base + PWR_CTRL2);
>  
> -	samsung_clk_sleep_init(reg_base, exynos5250_clk_regs,
> +	samsung_clk_sleep_init(reg_base, NULL, exynos5250_clk_regs,
>  			       ARRAY_SIZE(exynos5250_clk_regs));
>  	exynos5_subcmus_init(ctx, ARRAY_SIZE(exynos5250_subcmus),
>  			     exynos5250_subcmus);
> diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
> index a9df4e6db82fa7831d4e5c7210b0163d7d301ec1..1982e0751ceec7e57f9e82d96dcbadce1f691092 100644
> --- a/drivers/clk/samsung/clk-exynos5420.c
> +++ b/drivers/clk/samsung/clk-exynos5420.c
> @@ -1649,12 +1649,12 @@ static void __init exynos5x_clk_init(struct device_node *np,
>  				ARRAY_SIZE(exynos5800_cpu_clks));
>  	}
>  
> -	samsung_clk_extended_sleep_init(reg_base,
> +	samsung_clk_extended_sleep_init(reg_base, NULL,
>  		exynos5x_clk_regs, ARRAY_SIZE(exynos5x_clk_regs),
>  		exynos5420_set_clksrc, ARRAY_SIZE(exynos5420_set_clksrc));
>  
>  	if (soc == EXYNOS5800) {
> -		samsung_clk_sleep_init(reg_base, exynos5800_clk_regs,
> +		samsung_clk_sleep_init(reg_base, NULL, exynos5800_clk_regs,
>  				       ARRAY_SIZE(exynos5800_clk_regs));
>  
>  		exynos5_subcmus_init(ctx, ARRAY_SIZE(exynos5800_subcmus),
> diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c
> index dbc9925ca8f46e951dfb5d391c0e744ca370abcc..07b2948ae7ea48f126ab420be57d8c2705979464 100644
> --- a/drivers/clk/samsung/clk.c
> +++ b/drivers/clk/samsung/clk.c
> @@ -12,8 +12,10 @@
>  #include <linux/clkdev.h>
>  #include <linux/clk-provider.h>
>  #include <linux/io.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/of_address.h>
> +#include <linux/regmap.h>
>  #include <linux/syscore_ops.h>
>  
>  #include "clk.h"
> @@ -21,19 +23,29 @@
>  static LIST_HEAD(clock_reg_cache_list);
>  
>  void samsung_clk_save(void __iomem *base,
> +				    struct regmap *regmap,

Here it is regmap, in samsung_clk_extended_sleep_init() it is called
sysreg. This suggests that in some cases this replaces the MMIO - like
here in samsung_clk_save().


>  				    struct samsung_clk_reg_dump *rd,
>  				    unsigned int num_regs)
>  {
> -	for (; num_regs > 0; --num_regs, ++rd)
> -		rd->value = readl(base + rd->offset);
> +	for (; num_regs > 0; --num_regs, ++rd) {
> +		if (base)
> +			rd->value = readl(base + rd->offset);
> +		if (regmap)
> +			regmap_read(regmap, rd->offset, &rd->value);
> +	}
>  }
>  
>  void samsung_clk_restore(void __iomem *base,
> +				      struct regmap *regmap,
>  				      const struct samsung_clk_reg_dump *rd,
>  				      unsigned int num_regs)
>  {
> -	for (; num_regs > 0; --num_regs, ++rd)
> -		writel(rd->value, base + rd->offset);
> +	for (; num_regs > 0; --num_regs, ++rd) {
> +		if (base)
> +			writel(rd->value, base + rd->offset);
> +		if (regmap)
> +			regmap_write(regmap, rd->offset, rd->value);
> +	}
>  }
>  
>  struct samsung_clk_reg_dump *samsung_clk_alloc_reg_dump(
> @@ -227,6 +239,82 @@ void __init samsung_clk_register_div(struct samsung_clk_provider *ctx,
>  	}
>  }
>  
> +#define ACG_MSK GENMASK(6, 4)
> +#define CLK_IDLE GENMASK(5, 4)
> +static int samsung_auto_clk_gate_is_en(struct clk_hw *hw)
> +{
> +	u32 reg;
> +	struct clk_gate *gate = to_clk_gate(hw);
> +
> +	reg = readl(gate->reg);
> +	return ((reg & ACG_MSK) == CLK_IDLE) ? 0 : 1;
> +}
> +
> +/* enable and disable are nops in automatic clock mode */
> +static int samsung_auto_clk_gate_en(struct clk_hw *hw)
> +{
> +	return 0;
> +}
> +
> +static void samsung_auto_clk_gate_dis(struct clk_hw *hw)
> +{
> +}
> +
> +static const struct clk_ops samsung_auto_clk_gate_ops = {
> +	.enable = samsung_auto_clk_gate_en,
> +	.disable = samsung_auto_clk_gate_dis,
> +	.is_enabled = samsung_auto_clk_gate_is_en,
> +};
> +
> +struct clk_hw *samsung_register_auto_gate(struct device *dev,
> +		struct device_node *np, const char *name,
> +		const char *parent_name, const struct clk_hw *parent_hw,
> +		const struct clk_parent_data *parent_data,
> +		unsigned long flags,
> +		void __iomem *reg, u8 bit_idx,
> +		u8 clk_gate_flags, spinlock_t *lock)
> +{
> +	struct clk_gate *gate;
> +	struct clk_hw *hw;
> +	struct clk_init_data init = {};
> +	int ret = -EINVAL;
> +
> +	/* allocate the gate */
> +	gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> +	if (!gate)
> +		return ERR_PTR(-ENOMEM);
> +
> +	init.name = name;
> +	init.ops = &samsung_auto_clk_gate_ops;
> +	init.flags = flags;
> +	init.parent_names = parent_name ? &parent_name : NULL;
> +	init.parent_hws = parent_hw ? &parent_hw : NULL;
> +	init.parent_data = parent_data;
> +	if (parent_name || parent_hw || parent_data)
> +		init.num_parents = 1;
> +	else
> +		init.num_parents = 0;
> +
> +	/* struct clk_gate assignments */
> +	gate->reg = reg;
> +	gate->bit_idx = bit_idx;
> +	gate->flags = clk_gate_flags;
> +	gate->lock = lock;
> +	gate->hw.init = &init;
> +
> +	hw = &gate->hw;
> +	if (dev || !np)
> +		ret = clk_hw_register(dev, hw);
> +	else if (np)
> +		ret = of_clk_hw_register(np, hw);
> +	if (ret) {
> +		kfree(gate);
> +		hw = ERR_PTR(ret);
> +	}
> +
> +	return hw;
> +}
> +
>  /* register a list of gate clocks */
>  void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx,
>  				const struct samsung_gate_clock *list,
> @@ -234,11 +322,21 @@ void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx,
>  {
>  	struct clk_hw *clk_hw;
>  	unsigned int idx;
> +	void __iomem *reg_offs;
>  
>  	for (idx = 0; idx < nr_clk; idx++, list++) {
> -		clk_hw = clk_hw_register_gate(ctx->dev, list->name, list->parent_name,
> -				list->flags, ctx->reg_base + list->offset,
> +		reg_offs = ctx->reg_base + list->offset;
> +
> +		if (ctx->auto_clock_gate && ctx->gate_dbg_offset)
> +			clk_hw = samsung_register_auto_gate(ctx->dev, NULL,
> +				list->name, list->parent_name, NULL, NULL,
> +				list->flags, reg_offs + ctx->gate_dbg_offset,
>  				list->bit_idx, list->gate_flags, &ctx->lock);
> +		else
> +			clk_hw = clk_hw_register_gate(ctx->dev, list->name,
> +				list->parent_name, list->flags,
> +				ctx->reg_base + list->offset, list->bit_idx,
> +				list->gate_flags, &ctx->lock);
>  		if (IS_ERR(clk_hw)) {
>  			pr_err("%s: failed to register clock %s\n", __func__,
>  				list->name);
> @@ -276,10 +374,11 @@ static int samsung_clk_suspend(void)
>  	struct samsung_clock_reg_cache *reg_cache;
>  
>  	list_for_each_entry(reg_cache, &clock_reg_cache_list, node) {
> -		samsung_clk_save(reg_cache->reg_base, reg_cache->rdump,
> -				reg_cache->rd_num);
> -		samsung_clk_restore(reg_cache->reg_base, reg_cache->rsuspend,
> -				reg_cache->rsuspend_num);
> +		samsung_clk_save(reg_cache->reg_base, reg_cache->sysreg,
> +				 reg_cache->rdump, reg_cache->rd_num);
> +		samsung_clk_restore(reg_cache->reg_base, reg_cache->sysreg,
> +				    reg_cache->rsuspend,
> +				    reg_cache->rsuspend_num);
>  	}
>  	return 0;
>  }
> @@ -289,8 +388,8 @@ static void samsung_clk_resume(void)
>  	struct samsung_clock_reg_cache *reg_cache;
>  
>  	list_for_each_entry(reg_cache, &clock_reg_cache_list, node)
> -		samsung_clk_restore(reg_cache->reg_base, reg_cache->rdump,
> -				reg_cache->rd_num);
> +		samsung_clk_restore(reg_cache->reg_base, reg_cache->sysreg,
> +				    reg_cache->rdump, reg_cache->rd_num);
>  }
>  
>  static struct syscore_ops samsung_clk_syscore_ops = {
> @@ -299,6 +398,7 @@ static struct syscore_ops samsung_clk_syscore_ops = {
>  };
>  
>  void samsung_clk_extended_sleep_init(void __iomem *reg_base,
> +			struct regmap *sysreg,
>  			const unsigned long *rdump,
>  			unsigned long nr_rdump,
>  			const struct samsung_clk_reg_dump *rsuspend,
> @@ -319,6 +419,7 @@ void samsung_clk_extended_sleep_init(void __iomem *reg_base,
>  		register_syscore_ops(&samsung_clk_syscore_ops);
>  
>  	reg_cache->reg_base = reg_base;
> +	reg_cache->sysreg = sysreg;
>  	reg_cache->rd_num = nr_rdump;
>  	reg_cache->rsuspend = rsuspend;
>  	reg_cache->rsuspend_num = nr_rsuspend;
> @@ -334,6 +435,12 @@ void samsung_clk_extended_sleep_init(void __iomem *reg_base,
>  void __init samsung_cmu_register_clocks(struct samsung_clk_provider *ctx,
>  					const struct samsung_cmu_info *cmu)
>  {
> +	ctx->auto_clock_gate = cmu->auto_clock_gate;
> +	ctx->gate_dbg_offset = cmu->gate_dbg_offset;
> +	ctx->option_offset = cmu->option_offset;
> +	ctx->drcg_offset = cmu->drcg_offset;
> +	ctx->memclk_offset = cmu->memclk_offset;
> +
>  	if (cmu->pll_clks)
>  		samsung_clk_register_pll(ctx, cmu->pll_clks, cmu->nr_pll_clks);
>  	if (cmu->mux_clks)
> @@ -353,6 +460,31 @@ void __init samsung_cmu_register_clocks(struct samsung_clk_provider *ctx,
>  		samsung_clk_register_cpu(ctx, cmu->cpu_clks, cmu->nr_cpu_clks);
>  }
>  
> +/* Enable Dynamic Root Clock Gating of bus components*/
> +void samsung_en_dyn_root_clk_gating(struct device_node *np,
> +				    struct samsung_clk_provider *ctx,
> +				    const struct samsung_cmu_info *cmu)
> +{
> +	if (ctx && !ctx->auto_clock_gate)

Test for ctx should trigger Smatch warning.

> +		return;
> +
> +	ctx->sysreg = syscon_regmap_lookup_by_phandle(np, "samsung,sysreg");
> +	if (!IS_ERR_OR_NULL(ctx->sysreg)) {

Can it return NULL? anyway that's very confusing if() - negated OR_NULL.
Better just
if (IS_ERR()) {
	ctx->sysreg = NULL;
	return;
}




Best regards,
Krzysztof

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

* Re: [PATCH 8/9] clk: samsung: gs101: Enable auto_clock_gate mode for each gs101 CMU
  2025-10-13 20:51 ` [PATCH 8/9] clk: samsung: gs101: Enable auto_clock_gate mode for each gs101 CMU Peter Griffin
@ 2025-10-21 19:47   ` Krzysztof Kozlowski
  2025-10-21 21:03     ` Peter Griffin
  0 siblings, 1 reply; 30+ messages in thread
From: Krzysztof Kozlowski @ 2025-10-21 19:47 UTC (permalink / raw)
  To: Peter Griffin, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Alim Akhtar, André Draszik, Tudor Ambarus, Michael Turquette,
	Stephen Boyd, Sam Protsenko, Sylwester Nawrocki, Chanwoo Choi
  Cc: Will McVicker, devicetree, linux-arm-kernel, linux-samsung-soc,
	linux-kernel, linux-clk, Krzysztof Kozlowski, kernel-team

On 13/10/2025 22:51, Peter Griffin wrote:
> Enable auto clock mode, and define the additional fields which are used
> when this mode is enabled.
> 
> /sys/kernel/debug/clk/clk_summary now reports approximately 308 running
> clocks and 298 disabled clocks. Prior to this commit 586 clocks were
> running and 17 disabled. To ensure compatability with older DTs the

Typo

> resource size is checked and an error issued if the DT needs updating.

I fail to see how you keek it compatible. See further.

> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  drivers/clk/samsung/clk-gs101.c | 80 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
> 
> diff --git a/drivers/clk/samsung/clk-gs101.c b/drivers/clk/samsung/clk-gs101.c
> index 70b26db9b95ad0b376d23f637c7683fbc8c8c600..baf41ae6c9e2480cb83531acf7eae190c6aff819 100644
> --- a/drivers/clk/samsung/clk-gs101.c
> +++ b/drivers/clk/samsung/clk-gs101.c
> @@ -9,6 +9,7 @@
>  #include <linux/clk-provider.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/of.h>
> +#include <linux/of_address.h>
>  #include <linux/platform_device.h>
>  
>  #include <dt-bindings/clock/google,gs101.h>
> @@ -17,6 +18,8 @@
>  #include "clk-exynos-arm64.h"
>  #include "clk-pll.h"
>  
> +int check_cmu_res_size(struct device_node *np);
> +
>  /* NOTE: Must be equal to the last clock ID increased by one */
>  #define CLKS_NR_TOP	(CLK_GOUT_CMU_TPU_UART + 1)
>  #define CLKS_NR_APM	(CLK_APM_PLL_DIV16_APM + 1)
> @@ -26,6 +29,10 @@
>  #define CLKS_NR_PERIC0	(CLK_GOUT_PERIC0_SYSREG_PERIC0_PCLK + 1)
>  #define CLKS_NR_PERIC1	(CLK_GOUT_PERIC1_SYSREG_PERIC1_PCLK + 1)
>  
> +#define GS101_GATE_DBG_OFFSET 0x4000
> +#define GS101_DRCG_EN_OFFSET  0x104
> +#define GS101_MEMCLK_OFFSET   0x108
> +
>  /* ---- CMU_TOP ------------------------------------------------------------- */
>  
>  /* Register Offset definitions for CMU_TOP (0x1e080000) */
> @@ -1433,6 +1440,9 @@ static const struct samsung_cmu_info top_cmu_info __initconst = {
>  	.nr_clk_ids		= CLKS_NR_TOP,
>  	.clk_regs		= cmu_top_clk_regs,
>  	.nr_clk_regs		= ARRAY_SIZE(cmu_top_clk_regs),
> +	.auto_clock_gate	= true,
> +	.gate_dbg_offset	= GS101_GATE_DBG_OFFSET,
> +	.option_offset		= CMU_CMU_TOP_CONTROLLER_OPTION,
>  };
>  
>  static void __init gs101_cmu_top_init(struct device_node *np)
> @@ -1900,6 +1910,11 @@ static const struct samsung_gate_clock apm_gate_clks[] __initconst = {
>  	     CLK_CON_GAT_GOUT_BLK_APM_UID_XIU_DP_APM_IPCLKPORT_ACLK, 21, CLK_IS_CRITICAL, 0),
>  };
>  
> +static const unsigned long dcrg_memclk_sysreg[] __initconst = {
> +	GS101_DRCG_EN_OFFSET,
> +	GS101_MEMCLK_OFFSET,
> +};
> +
>  static const struct samsung_cmu_info apm_cmu_info __initconst = {
>  	.mux_clks		= apm_mux_clks,
>  	.nr_mux_clks		= ARRAY_SIZE(apm_mux_clks),
> @@ -1912,6 +1927,12 @@ static const struct samsung_cmu_info apm_cmu_info __initconst = {
>  	.nr_clk_ids		= CLKS_NR_APM,
>  	.clk_regs		= apm_clk_regs,
>  	.nr_clk_regs		= ARRAY_SIZE(apm_clk_regs),
> +	.sysreg_clk_regs	= dcrg_memclk_sysreg,
> +	.nr_sysreg_clk_regs	= ARRAY_SIZE(dcrg_memclk_sysreg),
> +	.auto_clock_gate	= true,
> +	.gate_dbg_offset	= GS101_GATE_DBG_OFFSET,
> +	.drcg_offset		= GS101_DRCG_EN_OFFSET,
> +	.memclk_offset		= GS101_MEMCLK_OFFSET,
>  };
>  
>  /* ---- CMU_HSI0 ------------------------------------------------------------ */
> @@ -2375,7 +2396,14 @@ static const struct samsung_cmu_info hsi0_cmu_info __initconst = {
>  	.nr_clk_ids		= CLKS_NR_HSI0,
>  	.clk_regs		= hsi0_clk_regs,
>  	.nr_clk_regs		= ARRAY_SIZE(hsi0_clk_regs),
> +	.sysreg_clk_regs	= dcrg_memclk_sysreg,
> +	.nr_sysreg_clk_regs	= ARRAY_SIZE(dcrg_memclk_sysreg),
>  	.clk_name		= "bus",
> +	.auto_clock_gate        = true,
> +	.gate_dbg_offset        = GS101_GATE_DBG_OFFSET,
> +	.option_offset		= HSI0_CMU_HSI0_CONTROLLER_OPTION,
> +	.drcg_offset		= GS101_DRCG_EN_OFFSET,
> +	.memclk_offset		= GS101_MEMCLK_OFFSET,
>  };
>  
>  /* ---- CMU_HSI2 ------------------------------------------------------------ */
> @@ -2863,7 +2891,14 @@ static const struct samsung_cmu_info hsi2_cmu_info __initconst = {
>  	.nr_clk_ids		= CLKS_NR_HSI2,
>  	.clk_regs		= cmu_hsi2_clk_regs,
>  	.nr_clk_regs		= ARRAY_SIZE(cmu_hsi2_clk_regs),
> +	.sysreg_clk_regs	= dcrg_memclk_sysreg,
> +	.nr_sysreg_clk_regs	= ARRAY_SIZE(dcrg_memclk_sysreg),
>  	.clk_name		= "bus",
> +	.auto_clock_gate        = true,
> +	.gate_dbg_offset        = GS101_GATE_DBG_OFFSET,
> +	.option_offset		= HSI2_CMU_HSI2_CONTROLLER_OPTION,
> +	.drcg_offset		= GS101_DRCG_EN_OFFSET,
> +	.memclk_offset		= GS101_MEMCLK_OFFSET,
>  };
>  
>  /* ---- CMU_MISC ------------------------------------------------------------ */
> @@ -3423,11 +3458,37 @@ static const struct samsung_cmu_info misc_cmu_info __initconst = {
>  	.nr_clk_ids		= CLKS_NR_MISC,
>  	.clk_regs		= misc_clk_regs,
>  	.nr_clk_regs		= ARRAY_SIZE(misc_clk_regs),
> +	.sysreg_clk_regs	= dcrg_memclk_sysreg,
> +	.nr_sysreg_clk_regs	= ARRAY_SIZE(dcrg_memclk_sysreg),
>  	.clk_name		= "bus",
> +	.auto_clock_gate	= true,
> +	.gate_dbg_offset	= GS101_GATE_DBG_OFFSET,
> +	.option_offset		= MISC_CMU_MISC_CONTROLLER_OPTION,
> +	.drcg_offset		= GS101_DRCG_EN_OFFSET,
> +	.memclk_offset		= GS101_MEMCLK_OFFSET,
>  };
>  
> +/* for old DT compatbility with incorrect CMU size*/
> +int check_cmu_res_size(struct device_node *np)
> +{
> +	struct resource res;
> +	resource_size_t size;
> +
> +	if (of_address_to_resource(np, 0, &res))
> +		return -ENODEV;
> +
> +	size = resource_size(&res);
> +	if (size != 0x10000) {
> +		pr_warn("%pOF: resource to small. Please update your DT\n", np);
> +		return -ENODEV;
> +	}
> +	return 0;
> +}
> +
>  static void __init gs101_cmu_misc_init(struct device_node *np)
>  {
> +	if (check_cmu_res_size(np))
> +		return;

You will not register CMU on old DTB.

>  	exynos_arm64_register_cmu(NULL, np, &misc_cmu_info);
>  }
>  
> @@ -4010,6 +4071,10 @@ static const struct samsung_gate_clock peric0_gate_clks[] __initconst = {
>  	     21, 0, 0),
>  };
>  
> +static const unsigned long dcrg_sysreg[] __initconst = {
> +	GS101_DRCG_EN_OFFSET,
> +};
> +
>  static const struct samsung_cmu_info peric0_cmu_info __initconst = {
>  	.mux_clks		= peric0_mux_clks,
>  	.nr_mux_clks		= ARRAY_SIZE(peric0_mux_clks),
> @@ -4020,7 +4085,13 @@ static const struct samsung_cmu_info peric0_cmu_info __initconst = {
>  	.nr_clk_ids		= CLKS_NR_PERIC0,
>  	.clk_regs		= peric0_clk_regs,
>  	.nr_clk_regs		= ARRAY_SIZE(peric0_clk_regs),
> +	.sysreg_clk_regs	= dcrg_sysreg,
> +	.nr_sysreg_clk_regs	= ARRAY_SIZE(dcrg_sysreg),
>  	.clk_name		= "bus",
> +	.auto_clock_gate        = true,
> +	.gate_dbg_offset        = GS101_GATE_DBG_OFFSET,
> +	.option_offset		= PERIC0_CMU_PERIC0_CONTROLLER_OPTION,
> +	.drcg_offset		= GS101_DRCG_EN_OFFSET,
>  };
>  
>  /* ---- CMU_PERIC1 ---------------------------------------------------------- */
> @@ -4368,7 +4439,13 @@ static const struct samsung_cmu_info peric1_cmu_info __initconst = {
>  	.nr_clk_ids		= CLKS_NR_PERIC1,
>  	.clk_regs		= peric1_clk_regs,
>  	.nr_clk_regs		= ARRAY_SIZE(peric1_clk_regs),
> +	.sysreg_clk_regs	= dcrg_sysreg,
> +	.nr_sysreg_clk_regs	= ARRAY_SIZE(dcrg_sysreg),
>  	.clk_name		= "bus",
> +	.auto_clock_gate        = true,
> +	.gate_dbg_offset        = GS101_GATE_DBG_OFFSET,
> +	.option_offset		= PERIC1_CMU_PERIC1_CONTROLLER_OPTION,
> +	.drcg_offset		= GS101_DRCG_EN_OFFSET,
>  };
>  
>  /* ---- platform_driver ----------------------------------------------------- */
> @@ -4378,6 +4455,9 @@ static int __init gs101_cmu_probe(struct platform_device *pdev)
>  	const struct samsung_cmu_info *info;
>  	struct device *dev = &pdev->dev;
>  
> +	if (check_cmu_res_size(dev->of_node))
> +		return -ENODEV;

And here you will really bail out with error.

> +
>  	info = of_device_get_match_data(dev);
>  	exynos_arm64_register_cmu(dev, dev->of_node, info);
>  
> 


Best regards,
Krzysztof

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

* Re: [PATCH 8/9] clk: samsung: gs101: Enable auto_clock_gate mode for each gs101 CMU
  2025-10-21 19:47   ` Krzysztof Kozlowski
@ 2025-10-21 21:03     ` Peter Griffin
  2025-10-22  5:45       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 30+ messages in thread
From: Peter Griffin @ 2025-10-21 21:03 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar,
	André Draszik, Tudor Ambarus, Michael Turquette,
	Stephen Boyd, Sam Protsenko, Sylwester Nawrocki, Chanwoo Choi,
	Will McVicker, devicetree, linux-arm-kernel, linux-samsung-soc,
	linux-kernel, linux-clk, Krzysztof Kozlowski, kernel-team

Hi Krzysztof,

Thanks for the review feedback.

On Tue, 21 Oct 2025 at 20:48, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 13/10/2025 22:51, Peter Griffin wrote:
> > Enable auto clock mode, and define the additional fields which are used
> > when this mode is enabled.
> >
> > /sys/kernel/debug/clk/clk_summary now reports approximately 308 running
> > clocks and 298 disabled clocks. Prior to this commit 586 clocks were
> > running and 17 disabled. To ensure compatability with older DTs the
>
> Typo

Will fix.

>
> > resource size is checked and an error issued if the DT needs updating.
>
> I fail to see how you keek it compatible. See further.
>
> >
> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > ---
> >  drivers/clk/samsung/clk-gs101.c | 80 +++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 80 insertions(+)
> >
> > diff --git a/drivers/clk/samsung/clk-gs101.c b/drivers/clk/samsung/clk-gs101.c
> > index 70b26db9b95ad0b376d23f637c7683fbc8c8c600..baf41ae6c9e2480cb83531acf7eae190c6aff819 100644
> > --- a/drivers/clk/samsung/clk-gs101.c
> > +++ b/drivers/clk/samsung/clk-gs101.c
> > @@ -9,6 +9,7 @@
> >  #include <linux/clk-provider.h>
> >  #include <linux/mod_devicetable.h>
> >  #include <linux/of.h>
> > +#include <linux/of_address.h>
> >  #include <linux/platform_device.h>
> >
> >  #include <dt-bindings/clock/google,gs101.h>
> > @@ -17,6 +18,8 @@
> >  #include "clk-exynos-arm64.h"
> >  #include "clk-pll.h"
> >
> > +int check_cmu_res_size(struct device_node *np);
> > +
> >  /* NOTE: Must be equal to the last clock ID increased by one */
> >  #define CLKS_NR_TOP  (CLK_GOUT_CMU_TPU_UART + 1)
> >  #define CLKS_NR_APM  (CLK_APM_PLL_DIV16_APM + 1)
> > @@ -26,6 +29,10 @@
> >  #define CLKS_NR_PERIC0       (CLK_GOUT_PERIC0_SYSREG_PERIC0_PCLK + 1)
> >  #define CLKS_NR_PERIC1       (CLK_GOUT_PERIC1_SYSREG_PERIC1_PCLK + 1)
> >
> > +#define GS101_GATE_DBG_OFFSET 0x4000
> > +#define GS101_DRCG_EN_OFFSET  0x104
> > +#define GS101_MEMCLK_OFFSET   0x108
> > +
> >  /* ---- CMU_TOP ------------------------------------------------------------- */
> >
> >  /* Register Offset definitions for CMU_TOP (0x1e080000) */
> > @@ -1433,6 +1440,9 @@ static const struct samsung_cmu_info top_cmu_info __initconst = {
> >       .nr_clk_ids             = CLKS_NR_TOP,
> >       .clk_regs               = cmu_top_clk_regs,
> >       .nr_clk_regs            = ARRAY_SIZE(cmu_top_clk_regs),
> > +     .auto_clock_gate        = true,
> > +     .gate_dbg_offset        = GS101_GATE_DBG_OFFSET,
> > +     .option_offset          = CMU_CMU_TOP_CONTROLLER_OPTION,
> >  };
> >
> >  static void __init gs101_cmu_top_init(struct device_node *np)
> > @@ -1900,6 +1910,11 @@ static const struct samsung_gate_clock apm_gate_clks[] __initconst = {
> >            CLK_CON_GAT_GOUT_BLK_APM_UID_XIU_DP_APM_IPCLKPORT_ACLK, 21, CLK_IS_CRITICAL, 0),
> >  };
> >
> > +static const unsigned long dcrg_memclk_sysreg[] __initconst = {
> > +     GS101_DRCG_EN_OFFSET,
> > +     GS101_MEMCLK_OFFSET,
> > +};
> > +
> >  static const struct samsung_cmu_info apm_cmu_info __initconst = {
> >       .mux_clks               = apm_mux_clks,
> >       .nr_mux_clks            = ARRAY_SIZE(apm_mux_clks),
> > @@ -1912,6 +1927,12 @@ static const struct samsung_cmu_info apm_cmu_info __initconst = {
> >       .nr_clk_ids             = CLKS_NR_APM,
> >       .clk_regs               = apm_clk_regs,
> >       .nr_clk_regs            = ARRAY_SIZE(apm_clk_regs),
> > +     .sysreg_clk_regs        = dcrg_memclk_sysreg,
> > +     .nr_sysreg_clk_regs     = ARRAY_SIZE(dcrg_memclk_sysreg),
> > +     .auto_clock_gate        = true,
> > +     .gate_dbg_offset        = GS101_GATE_DBG_OFFSET,
> > +     .drcg_offset            = GS101_DRCG_EN_OFFSET,
> > +     .memclk_offset          = GS101_MEMCLK_OFFSET,
> >  };
> >
> >  /* ---- CMU_HSI0 ------------------------------------------------------------ */
> > @@ -2375,7 +2396,14 @@ static const struct samsung_cmu_info hsi0_cmu_info __initconst = {
> >       .nr_clk_ids             = CLKS_NR_HSI0,
> >       .clk_regs               = hsi0_clk_regs,
> >       .nr_clk_regs            = ARRAY_SIZE(hsi0_clk_regs),
> > +     .sysreg_clk_regs        = dcrg_memclk_sysreg,
> > +     .nr_sysreg_clk_regs     = ARRAY_SIZE(dcrg_memclk_sysreg),
> >       .clk_name               = "bus",
> > +     .auto_clock_gate        = true,
> > +     .gate_dbg_offset        = GS101_GATE_DBG_OFFSET,
> > +     .option_offset          = HSI0_CMU_HSI0_CONTROLLER_OPTION,
> > +     .drcg_offset            = GS101_DRCG_EN_OFFSET,
> > +     .memclk_offset          = GS101_MEMCLK_OFFSET,
> >  };
> >
> >  /* ---- CMU_HSI2 ------------------------------------------------------------ */
> > @@ -2863,7 +2891,14 @@ static const struct samsung_cmu_info hsi2_cmu_info __initconst = {
> >       .nr_clk_ids             = CLKS_NR_HSI2,
> >       .clk_regs               = cmu_hsi2_clk_regs,
> >       .nr_clk_regs            = ARRAY_SIZE(cmu_hsi2_clk_regs),
> > +     .sysreg_clk_regs        = dcrg_memclk_sysreg,
> > +     .nr_sysreg_clk_regs     = ARRAY_SIZE(dcrg_memclk_sysreg),
> >       .clk_name               = "bus",
> > +     .auto_clock_gate        = true,
> > +     .gate_dbg_offset        = GS101_GATE_DBG_OFFSET,
> > +     .option_offset          = HSI2_CMU_HSI2_CONTROLLER_OPTION,
> > +     .drcg_offset            = GS101_DRCG_EN_OFFSET,
> > +     .memclk_offset          = GS101_MEMCLK_OFFSET,
> >  };
> >
> >  /* ---- CMU_MISC ------------------------------------------------------------ */
> > @@ -3423,11 +3458,37 @@ static const struct samsung_cmu_info misc_cmu_info __initconst = {
> >       .nr_clk_ids             = CLKS_NR_MISC,
> >       .clk_regs               = misc_clk_regs,
> >       .nr_clk_regs            = ARRAY_SIZE(misc_clk_regs),
> > +     .sysreg_clk_regs        = dcrg_memclk_sysreg,
> > +     .nr_sysreg_clk_regs     = ARRAY_SIZE(dcrg_memclk_sysreg),
> >       .clk_name               = "bus",
> > +     .auto_clock_gate        = true,
> > +     .gate_dbg_offset        = GS101_GATE_DBG_OFFSET,
> > +     .option_offset          = MISC_CMU_MISC_CONTROLLER_OPTION,
> > +     .drcg_offset            = GS101_DRCG_EN_OFFSET,
> > +     .memclk_offset          = GS101_MEMCLK_OFFSET,
> >  };
> >
> > +/* for old DT compatbility with incorrect CMU size*/
> > +int check_cmu_res_size(struct device_node *np)
> > +{
> > +     struct resource res;
> > +     resource_size_t size;
> > +
> > +     if (of_address_to_resource(np, 0, &res))
> > +             return -ENODEV;
> > +
> > +     size = resource_size(&res);
> > +     if (size != 0x10000) {
> > +             pr_warn("%pOF: resource to small. Please update your DT\n", np);
> > +             return -ENODEV;
> > +     }
> > +     return 0;
> > +}
> > +
> >  static void __init gs101_cmu_misc_init(struct device_node *np)
> >  {
> > +     if (check_cmu_res_size(np))
> > +             return;
>
> You will not register CMU on old DTB.

By "compatible" I meant the driver detects an old DTB with an
incorrect reg size and issues an error message on the console to
update your DT (as opposed to crashing trying to access a register
that hasn't been mapped).

Is it enough to re-word the commit message to make it clearer what will happen?

An alternative might be to try registering all the gates in manual
mode, but that seems like it would add more complexity for not much
benefit. It would also require that clk_ignore_unused kernel parameter
to have been passed (as manual clock mode has never worked without it)
and whilst it might boot today I imagine it would bitrot fast as
additional CMUs are added (and thus probably crash in a much more
obscure way).

Peter

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

* Re: [PATCH 8/9] clk: samsung: gs101: Enable auto_clock_gate mode for each gs101 CMU
  2025-10-21 21:03     ` Peter Griffin
@ 2025-10-22  5:45       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 30+ messages in thread
From: Krzysztof Kozlowski @ 2025-10-22  5:45 UTC (permalink / raw)
  To: Peter Griffin, Krzysztof Kozlowski
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar,
	André Draszik, Tudor Ambarus, Michael Turquette,
	Stephen Boyd, Sam Protsenko, Sylwester Nawrocki, Chanwoo Choi,
	Will McVicker, devicetree, linux-arm-kernel, linux-samsung-soc,
	linux-kernel, linux-clk, kernel-team

On 21/10/2025 23:03, Peter Griffin wrote:
>>> +}
>>> +
>>>  static void __init gs101_cmu_misc_init(struct device_node *np)
>>>  {
>>> +     if (check_cmu_res_size(np))
>>> +             return;
>>
>> You will not register CMU on old DTB.
> 
> By "compatible" I meant the driver detects an old DTB with an
> incorrect reg size and issues an error message on the console to
> update your DT (as opposed to crashing trying to access a register
> that hasn't been mapped).


That's not being compatible. You should not break anything here if size
is smaller - just fallback to previous manual mode.

> 
> Is it enough to re-word the commit message to make it clearer what will happen?
> 
> An alternative might be to try registering all the gates in manual
> mode, but that seems like it would add more complexity for not much
> benefit. It would also require that clk_ignore_unused kernel parameter
> to have been passed (as manual clock mode has never worked without it)
> and whilst it might boot today I imagine it would bitrot fast as
> additional CMUs are added (and thus probably crash in a much more
> obscure way).
> 
> Peter

Best regards,
Krzysztof


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

* Re: [PATCH 7/9] clk: samsung: Implement automatic clock gating mode for CMUs
  2025-10-16  9:40   ` André Draszik
@ 2025-10-28  8:50     ` Peter Griffin
  0 siblings, 0 replies; 30+ messages in thread
From: Peter Griffin @ 2025-10-28  8:50 UTC (permalink / raw)
  To: André Draszik
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar,
	Tudor Ambarus, Michael Turquette, Stephen Boyd, Sam Protsenko,
	Sylwester Nawrocki, Chanwoo Choi, Will McVicker,
	Krzysztof Kozlowski, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-clk, Krzysztof Kozlowski,
	kernel-team

Hi André,

Thanks for the review feedback!

On Thu, 16 Oct 2025 at 10:40, André Draszik <andre.draszik@linaro.org> wrote:
>
> Hi Peter,
>
> On Mon, 2025-10-13 at 21:51 +0100, Peter Griffin wrote:
> > Update exynos_arm64_init_clocks() so that it enables the automatic clock
> > mode bits in the CMU option register if the auto_clock_gate flag and
> > option_offset fields are set for the CMU.
> >
> > The CMU option register bits are global and effect every clock component in
> > the CMU, as such clearing the GATE_ENABLE_HWACG bit and setting GATE_MANUAL
> > bit on every gate register is only required if auto_clock_gate is false.
> >
> > Additionally if auto_clock_gate is enabled the dynamic root clock gating
> > and memclk registers will be configured in the corresponding CMUs sysreg
> > bank. These registers are exposed via syscon, so the register
> > suspend/resume paths are updated to handle using a regmap.
> >
> > As many gates for various Samsung SoCs are already exposed in the Samsung
> > clock drivers a new samsung_auto_clk_gate_ops is implemented. This uses
> > some CMU debug registers to report whether clocks are enabled or disabled
> > when operating in automatic mode. This allows
> > /sys/kernel/debug/clk/clk_summary to still dump the entire clock tree and
> > correctly report the status of each clock in the system.
> >
> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
>
> It's great to see some effort on this topic as this makes the clock handling
> on new Exynos much easier and less error prone and time-consuming, and having
> the hardware decide which clocks need to be enabled at a given point in time
> allows for a significantly more dynamic environment.
>
> Just some initial comments below.
>
> > ---
> >  drivers/clk/samsung/clk-exynos-arm64.c   |  47 +++++++--
> >  drivers/clk/samsung/clk-exynos4.c        |   6 +-
> >  drivers/clk/samsung/clk-exynos4412-isp.c |   4 +-
> >  drivers/clk/samsung/clk-exynos5250.c     |   2 +-
> >  drivers/clk/samsung/clk-exynos5420.c     |   4 +-
> >  drivers/clk/samsung/clk.c                | 161 ++++++++++++++++++++++++++++---
> >  drivers/clk/samsung/clk.h                |  49 +++++++++-
> >  7 files changed, 244 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/clk/samsung/clk-exynos-arm64.c b/drivers/clk/samsung/clk-exynos-arm64.c
> > index bf7de21f329ec89069dcf817ca578fcf9b2d9809..c302c836e8f9f6270753d86d7d986c88e6762f4f 100644
> > --- a/drivers/clk/samsung/clk-exynos-arm64.c
> > +++ b/drivers/clk/samsung/clk-exynos-arm64.c
> > @@ -24,6 +24,16 @@
> >  #define GATE_MANUAL          BIT(20)
> >  #define GATE_ENABLE_HWACG    BIT(28)
> >
> > +/* Option register bits */
> > +#define OPT_EN_DBG                   BIT(31)
> > +#define OPT_UNKNOWN                  BIT(30)
>
> Except for AOC, bit 30 is called ENABLE_LAYER2_CTRL in all gs101 CMUs.

Will fix.

>
> > +#define OPT_EN_PWR_MANAGEMENT                BIT(29)
> > +#define OPT_EN_AUTO_GATING           BIT(28)
> > +#define OPT_EN_MEM_PM_GATING         BIT(24)
> > +
> > +#define CMU_OPT_GLOBAL_EN_AUTO_GATING        (OPT_EN_DBG | OPT_UNKNOWN | \
> > +     OPT_EN_PWR_MANAGEMENT | OPT_EN_AUTO_GATING | OPT_EN_MEM_PM_GATING)
> > +
> >  /* PLL_CONx_PLL register offsets range */
> >  #define PLL_CON_OFF_START    0x100
> >  #define PLL_CON_OFF_END              0x600
> > @@ -37,6 +47,8 @@ struct exynos_arm64_cmu_data {
> >       unsigned int nr_clk_save;
> >       const struct samsung_clk_reg_dump *clk_suspend;
> >       unsigned int nr_clk_suspend;
> > +     struct samsung_clk_reg_dump *clk_sysreg_save;
> > +     unsigned int nr_clk_sysreg;
> >
> >       struct clk *clk;
> >       struct clk **pclks;
> > @@ -82,13 +94,28 @@ static void __init exynos_arm64_init_clocks(struct device_node *np,
> >       if (!reg_base)
> >               panic("%s: failed to map registers\n", __func__);
> >
> > +     if (cmu->auto_clock_gate && cmu->option_offset) {
> > +             /*
> > +              * Enable the global automatic mode for the entire CMU.
> > +              * This overrides the individual HWACG bits in each of the
> > +              * individual gate, mux and qch registers.
> > +              */
> > +             writel(CMU_OPT_GLOBAL_EN_AUTO_GATING,
> > +                    reg_base + cmu->option_offset);
> > +     }
> > +
> >       for (i = 0; i < reg_offs_len; ++i) {
> >               void __iomem *reg = reg_base + reg_offs[i];
> >               u32 val;
> >
> >               if (cmu->manual_plls && is_pll_con1_reg(reg_offs[i])) {
> >                       writel(PLL_CON1_MANUAL, reg);
> > -             } else if (is_gate_reg(reg_offs[i])) {
> > +             } else if (is_gate_reg(reg_offs[i]) && !cmu->auto_clock_gate) {
> > +                     /*
> > +                      * Setting GATE_MANUAL bit (which is described in TRM as
> > +                      * reserved!) overrides the global CMU automatic mode
> > +                      * option.
> > +                      */
> >                       val = readl(reg);
> >                       val |= GATE_MANUAL;
> >                       val &= ~GATE_ENABLE_HWACG;
> > @@ -219,7 +246,7 @@ void __init exynos_arm64_register_cmu(struct device *dev,
> >   * Return: 0 on success, or negative error code on error.
> >   */
> >  int __init exynos_arm64_register_cmu_pm(struct platform_device *pdev,
> > -                                     bool set_manual)
> > +                                     bool init_clk_regs)
>
> kerneldoc needs updating

Good spot. Will fix

>
> >  {
> >       const struct samsung_cmu_info *cmu;
> >       struct device *dev = &pdev->dev;
> > @@ -249,7 +276,7 @@ int __init exynos_arm64_register_cmu_pm(struct platform_device *pdev,
> >               dev_err(dev, "%s: could not enable bus clock %s; err = %d\n",
> >                      __func__, cmu->clk_name, ret);
> >
> > -     if (set_manual)
> > +     if (init_clk_regs)
> >               exynos_arm64_init_clocks(np, cmu);
> >
> >       reg_base = devm_platform_ioremap_resource(pdev, 0);
> > @@ -280,14 +307,18 @@ int exynos_arm64_cmu_suspend(struct device *dev)
> >       struct exynos_arm64_cmu_data *data = dev_get_drvdata(dev);
> >       int i;
> >
> > -     samsung_clk_save(data->ctx->reg_base, data->clk_save,
> > +     samsung_clk_save(data->ctx->reg_base, NULL, data->clk_save,
> >                        data->nr_clk_save);
> >
> > +     if (data->ctx->sysreg)
> > +             samsung_clk_save(NULL, data->ctx->sysreg, data->clk_save,
> > +                              data->nr_clk_save);
> > +
>
> I think this should be
>
> +               samsung_clk_save(NULL, data->ctx->sysreg, data->clk_sysreg_save,
> +                                data->nr_clk_sysreg);

Yes, you're correct! Will fix.
>
> ?
>
> >       for (i = 0; i < data->nr_pclks; i++)
> >               clk_prepare_enable(data->pclks[i]);
> >
> >       /* For suspend some registers have to be set to certain values */
> > -     samsung_clk_restore(data->ctx->reg_base, data->clk_suspend,
> > +     samsung_clk_restore(data->ctx->reg_base, NULL, data->clk_suspend,
> >                           data->nr_clk_suspend);
> >
> >       for (i = 0; i < data->nr_pclks; i++)
> > @@ -308,9 +339,13 @@ int exynos_arm64_cmu_resume(struct device *dev)
> >       for (i = 0; i < data->nr_pclks; i++)
> >               clk_prepare_enable(data->pclks[i]);
> >
> > -     samsung_clk_restore(data->ctx->reg_base, data->clk_save,
> > +     samsung_clk_restore(data->ctx->reg_base, NULL, data->clk_save,
> >                           data->nr_clk_save);
> >
> > +     if (data->ctx->sysreg)
> > +             samsung_clk_restore(NULL, data->ctx->sysreg, data->clk_save,
> > +                                 data->nr_clk_save);
> > +
>
> dito.

Will fix.

>
> >       for (i = 0; i < data->nr_pclks; i++)
> >               clk_disable_unprepare(data->pclks[i]);
> >
>
> [...]
>
> > diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c
> > index dbc9925ca8f46e951dfb5d391c0e744ca370abcc..07b2948ae7ea48f126ab420be57d8c2705979464 100644
> > --- a/drivers/clk/samsung/clk.c
> > +++ b/drivers/clk/samsung/clk.c
> > @@ -12,8 +12,10 @@
> >  #include <linux/clkdev.h>
> >  #include <linux/clk-provider.h>
> >  #include <linux/io.h>
> > +#include <linux/mfd/syscon.h>
> >  #include <linux/mod_devicetable.h>
> >  #include <linux/of_address.h>
> > +#include <linux/regmap.h>
> >  #include <linux/syscore_ops.h>
> >
> >  #include "clk.h"
> > @@ -21,19 +23,29 @@
> >  static LIST_HEAD(clock_reg_cache_list);
> >
> >  void samsung_clk_save(void __iomem *base,
> > +                                 struct regmap *regmap,
> >                                   struct samsung_clk_reg_dump *rd,
> >                                   unsigned int num_regs)
> >  {
> > -     for (; num_regs > 0; --num_regs, ++rd)
> > -             rd->value = readl(base + rd->offset);
> > +     for (; num_regs > 0; --num_regs, ++rd) {
> > +             if (base)
> > +                     rd->value = readl(base + rd->offset);
> > +             if (regmap)
>
> Should this be else if?

I will update it like you suggest to protect against that
>
> > +                     regmap_read(regmap, rd->offset, &rd->value);
>
> Otherwise users that (incorrectly) pass both base and regmap would
> cause this to end up reading the offset from the wrong memory
> region.
>
> At least this way that won't happen, but maybe this constrains should
> be made even more explicit.
>
> > +     }
> >  }
> >
> >  void samsung_clk_restore(void __iomem *base,
> > +                                   struct regmap *regmap,
> >                                     const struct samsung_clk_reg_dump *rd,
> >                                     unsigned int num_regs)
> >  {
> > -     for (; num_regs > 0; --num_regs, ++rd)
> > -             writel(rd->value, base + rd->offset);
> > +     for (; num_regs > 0; --num_regs, ++rd) {
> > +             if (base)
> > +                     writel(rd->value, base + rd->offset);
> > +             if (regmap)
> > +                     regmap_write(regmap, rd->offset, rd->value);
>
> dito.

Will fix.
>
> > +     }
> >  }
> >
> >  struct samsung_clk_reg_dump *samsung_clk_alloc_reg_dump(
> > @@ -227,6 +239,82 @@ void __init samsung_clk_register_div(struct samsung_clk_provider *ctx,
> >       }
> >  }
> >
> > +#define ACG_MSK GENMASK(6, 4)
> > +#define CLK_IDLE GENMASK(5, 4)
> > +static int samsung_auto_clk_gate_is_en(struct clk_hw *hw)
> > +{
> > +     u32 reg;
> > +     struct clk_gate *gate = to_clk_gate(hw);
> > +
> > +     reg = readl(gate->reg);
> > +     return ((reg & ACG_MSK) == CLK_IDLE) ? 0 : 1;
> > +}
> > +
> > +/* enable and disable are nops in automatic clock mode */
> > +static int samsung_auto_clk_gate_en(struct clk_hw *hw)
> > +{
> > +     return 0;
> > +}
> > +
> > +static void samsung_auto_clk_gate_dis(struct clk_hw *hw)
> > +{
> > +}
> > +
> > +static const struct clk_ops samsung_auto_clk_gate_ops = {
> > +     .enable = samsung_auto_clk_gate_en,
> > +     .disable = samsung_auto_clk_gate_dis,
> > +     .is_enabled = samsung_auto_clk_gate_is_en,
> > +};
> > +
> > +struct clk_hw *samsung_register_auto_gate(struct device *dev,
> > +             struct device_node *np, const char *name,
> > +             const char *parent_name, const struct clk_hw *parent_hw,
> > +             const struct clk_parent_data *parent_data,
> > +             unsigned long flags,
> > +             void __iomem *reg, u8 bit_idx,
> > +             u8 clk_gate_flags, spinlock_t *lock)
> > +{
> > +     struct clk_gate *gate;
> > +     struct clk_hw *hw;
> > +     struct clk_init_data init = {};
> > +     int ret = -EINVAL;
> > +
> > +     /* allocate the gate */
> > +     gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> > +     if (!gate)
> > +             return ERR_PTR(-ENOMEM);
> > +
> > +     init.name = name;
> > +     init.ops = &samsung_auto_clk_gate_ops;
> > +     init.flags = flags;
> > +     init.parent_names = parent_name ? &parent_name : NULL;
> > +     init.parent_hws = parent_hw ? &parent_hw : NULL;
> > +     init.parent_data = parent_data;
> > +     if (parent_name || parent_hw || parent_data)
> > +             init.num_parents = 1;
> > +     else
> > +             init.num_parents = 0;
> > +
> > +     /* struct clk_gate assignments */
> > +     gate->reg = reg;
> > +     gate->bit_idx = bit_idx;
> > +     gate->flags = clk_gate_flags;
> > +     gate->lock = lock;
> > +     gate->hw.init = &init;
> > +
> > +     hw = &gate->hw;
> > +     if (dev || !np)
> > +             ret = clk_hw_register(dev, hw);
> > +     else if (np)
> > +             ret = of_clk_hw_register(np, hw);
> > +     if (ret) {
> > +             kfree(gate);
> > +             hw = ERR_PTR(ret);
> > +     }
> > +
> > +     return hw;
> > +}
> > +
> >  /* register a list of gate clocks */
> >  void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx,
> >                               const struct samsung_gate_clock *list,
> > @@ -234,11 +322,21 @@ void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx,
> >  {
> >       struct clk_hw *clk_hw;
> >       unsigned int idx;
> > +     void __iomem *reg_offs;
> >
> >       for (idx = 0; idx < nr_clk; idx++, list++) {
> > -             clk_hw = clk_hw_register_gate(ctx->dev, list->name, list->parent_name,
> > -                             list->flags, ctx->reg_base + list->offset,
> > +             reg_offs = ctx->reg_base + list->offset;
> > +
> > +             if (ctx->auto_clock_gate && ctx->gate_dbg_offset)
> > +                     clk_hw = samsung_register_auto_gate(ctx->dev, NULL,
> > +                             list->name, list->parent_name, NULL, NULL,
> > +                             list->flags, reg_offs + ctx->gate_dbg_offset,
> >                               list->bit_idx, list->gate_flags, &ctx->lock);
> > +             else
> > +                     clk_hw = clk_hw_register_gate(ctx->dev, list->name,
> > +                             list->parent_name, list->flags,
> > +                             ctx->reg_base + list->offset, list->bit_idx,
> > +                             list->gate_flags, &ctx->lock);
> >               if (IS_ERR(clk_hw)) {
> >                       pr_err("%s: failed to register clock %s\n", __func__,
> >                               list->name);
>
> Maybe the error message should include the actual error clk_hw here?

Will update as you suggest.

>
> > @@ -276,10 +374,11 @@ static int samsung_clk_suspend(void)
> >       struct samsung_clock_reg_cache *reg_cache;
> >
> >       list_for_each_entry(reg_cache, &clock_reg_cache_list, node) {
> > -             samsung_clk_save(reg_cache->reg_base, reg_cache->rdump,
> > -                             reg_cache->rd_num);
> > -             samsung_clk_restore(reg_cache->reg_base, reg_cache->rsuspend,
> > -                             reg_cache->rsuspend_num);
> > +             samsung_clk_save(reg_cache->reg_base, reg_cache->sysreg,
> > +                              reg_cache->rdump, reg_cache->rd_num);
> > +             samsung_clk_restore(reg_cache->reg_base, reg_cache->sysreg,
> > +                                 reg_cache->rsuspend,
> > +                                 reg_cache->rsuspend_num);
> >       }
> >       return 0;
> >  }
> > @@ -289,8 +388,8 @@ static void samsung_clk_resume(void)
> >       struct samsung_clock_reg_cache *reg_cache;
> >
> >       list_for_each_entry(reg_cache, &clock_reg_cache_list, node)
> > -             samsung_clk_restore(reg_cache->reg_base, reg_cache->rdump,
> > -                             reg_cache->rd_num);
> > +             samsung_clk_restore(reg_cache->reg_base, reg_cache->sysreg,
> > +                                 reg_cache->rdump, reg_cache->rd_num);
> >  }
> >
> >  static struct syscore_ops samsung_clk_syscore_ops = {
> > @@ -299,6 +398,7 @@ static struct syscore_ops samsung_clk_syscore_ops = {
> >  };
> >
> >  void samsung_clk_extended_sleep_init(void __iomem *reg_base,
> > +                     struct regmap *sysreg,
> >                       const unsigned long *rdump,
> >                       unsigned long nr_rdump,
> >                       const struct samsung_clk_reg_dump *rsuspend,
> > @@ -319,6 +419,7 @@ void samsung_clk_extended_sleep_init(void __iomem *reg_base,
> >               register_syscore_ops(&samsung_clk_syscore_ops);
> >
> >       reg_cache->reg_base = reg_base;
> > +     reg_cache->sysreg = sysreg;
> >       reg_cache->rd_num = nr_rdump;
> >       reg_cache->rsuspend = rsuspend;
> >       reg_cache->rsuspend_num = nr_rsuspend;
> > @@ -334,6 +435,12 @@ void samsung_clk_extended_sleep_init(void __iomem *reg_base,
> >  void __init samsung_cmu_register_clocks(struct samsung_clk_provider *ctx,
> >                                       const struct samsung_cmu_info *cmu)
> >  {
> > +     ctx->auto_clock_gate = cmu->auto_clock_gate;
> > +     ctx->gate_dbg_offset = cmu->gate_dbg_offset;
> > +     ctx->option_offset = cmu->option_offset;
> > +     ctx->drcg_offset = cmu->drcg_offset;
> > +     ctx->memclk_offset = cmu->memclk_offset;
> > +
> >       if (cmu->pll_clks)
> >               samsung_clk_register_pll(ctx, cmu->pll_clks, cmu->nr_pll_clks);
> >       if (cmu->mux_clks)
> > @@ -353,6 +460,31 @@ void __init samsung_cmu_register_clocks(struct samsung_clk_provider *ctx,
> >               samsung_clk_register_cpu(ctx, cmu->cpu_clks, cmu->nr_cpu_clks);
> >  }
> >
> > +/* Enable Dynamic Root Clock Gating of bus components*/
>
> missing space before */

will fix.

>
> > +void samsung_en_dyn_root_clk_gating(struct device_node *np,
> > +                                 struct samsung_clk_provider *ctx,
> > +                                 const struct samsung_cmu_info *cmu)
> > +{
> > +     if (ctx && !ctx->auto_clock_gate)
>
> ctx can not be NULL here. If it could, then the following lines would also
> need updating, but again, it can not be NULL, if ctx allocation fails, the
> code will panic() before this here is reached.

I'll remove that check.
>
> > +             return;
> > +
> > +     ctx->sysreg = syscon_regmap_lookup_by_phandle(np, "samsung,sysreg");
> > +     if (!IS_ERR_OR_NULL(ctx->sysreg)) {
> > +             regmap_write(ctx->sysreg, ctx->drcg_offset, 0xffffffff);
>
> What does 0xffffffff mean? Maybe a macro or at least a comment would help.

Setting a bit "enables dynamic root clock gating (drcg)". Different
bits in the register correspond to different bus components. So this
is enabling drcg for everything.

>
> > +             /* not every sysreg controller has memclk reg*/
>
> missing space before */

Will fix
>
> Can you please check all such comments, since it seems to be recurring :-)

Will check!
>
> +               if (ctx->memclk_offset)
> > +                     regmap_write_bits(ctx->sysreg, ctx->memclk_offset, 0x1, 0x0);
> > +
> > +             samsung_clk_extended_sleep_init(NULL, ctx->sysreg,
> > +                                             cmu->sysreg_clk_regs,
> > +                                             cmu->nr_sysreg_clk_regs,
> > +                                             NULL, 0);
> > +     } else {
> > +             pr_warn("%pOF: Unable to get CMU sysreg\n", np);
> > +             ctx->sysreg = NULL;
> > +     }
> > +}
> > +
> >  /*
> >   * Common function which registers plls, muxes, dividers and gates
> >   * for each CMU. It also add CMU register list to register cache.
> > @@ -374,11 +506,14 @@ struct samsung_clk_provider * __init samsung_cmu_register_one(
> >       samsung_cmu_register_clocks(ctx, cmu);
> >
> >       if (cmu->clk_regs)
> > -             samsung_clk_extended_sleep_init(reg_base,
> > +             samsung_clk_extended_sleep_init(reg_base, NULL,
> >                       cmu->clk_regs, cmu->nr_clk_regs,
> >                       cmu->suspend_regs, cmu->nr_suspend_regs);
> >
> >       samsung_clk_of_add_provider(np, ctx);
> >
> > +     /* sysreg DT nodes reference a clock in this CMU */
> > +     samsung_en_dyn_root_clk_gating(np, ctx, cmu);
> > +
>
> samsung_cmu_register_one() is not called when the clocks are registered
> using exynos_arm64_register_cmu_pm(). Does the pm version need a call to
> samsung_en_dyn_root_clk_gating() somewhere?

Yes, good point, I will add it in v2. That should make things smoother
when we switch to the exynos_arm64_register_cmu_pm() version.

>
> >       return ctx;
> >  }
> > diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
> > index 18660c1ac6f0106b17b9efc9c6b3cd62d46f7b82..b719e057f45489e9d92ba54031fe633a8c9264ce 100644
> > --- a/drivers/clk/samsung/clk.h
> > +++ b/drivers/clk/samsung/clk.h
> > @@ -12,6 +12,7 @@
> >
> >  #include <linux/clk-provider.h>
> >  #include <linux/mod_devicetable.h>
> > +#include <linux/regmap.h>
> >  #include "clk-pll.h"
> >  #include "clk-cpu.h"
> >
> > @@ -19,13 +20,25 @@
> >   * struct samsung_clk_provider - information about clock provider
> >   * @reg_base: virtual address for the register base
> >   * @dev: clock provider device needed for runtime PM
> > + * @sysreg: syscon regmap for clock-provider sysreg controller
> >   * @lock: maintains exclusion between callbacks for a given clock-provider
> > + * @auto_clock_gate: enable auto clk mode for all clocks in clock-provider
> > + * @gate_dbg_offset: gate debug reg offset. Used for all gates in auto clk mode
> > + * @option_offset: option reg offset. Enables auto mode for clock-provider
> > + * @drcg_offset: dynamic root clk gate enable register offset
> > + * @memclk_offset: memclk enable register offset
>
> The last two are in the sysreg space, maybe the comment should mention that.

Will update the comment.

>
> >   * @clk_data: holds clock related data like clk_hw* and number of clocks
> >   */
> >  struct samsung_clk_provider {
> >       void __iomem *reg_base;
> >       struct device *dev;
> > +     struct regmap *sysreg;
> >       spinlock_t lock;
> > +     bool auto_clock_gate;
> > +     u32 gate_dbg_offset;
> > +     u32 option_offset;
> > +     u32 drcg_offset;
> > +     u32 memclk_offset;
> >       /* clk_data must be the last entry due to variable length 'hws' array */
> >       struct clk_hw_onecell_data clk_data;
> >  };
> > @@ -310,6 +323,7 @@ struct samsung_cpu_clock {
> >  struct samsung_clock_reg_cache {
> >       struct list_head node;
> >       void __iomem *reg_base;
> > +     struct regmap *sysreg;
> >       struct samsung_clk_reg_dump *rdump;
> >       unsigned int rd_num;
> >       const struct samsung_clk_reg_dump *rsuspend;
> > @@ -338,7 +352,14 @@ struct samsung_clock_reg_cache {
> >   * @suspend_regs: list of clock registers to set before suspend
> >   * @nr_suspend_regs: count of clock registers in @suspend_regs
> >   * @clk_name: name of the parent clock needed for CMU register access
> > + * @sysreg_clk_regs: list of sysreg clock registers
> > + * @nr_sysreg_clk_regs: count of clock registers in @sysreg_clk_regs
> >   * @manual_plls: Enable manual control for PLL clocks
> > + * @auto_clock_gate: enable auto clock mode for all components in CMU
> > + * @gate_dbg_offset: gate debug reg offset. Used by all gates in auto clk mode
> > + * @option_offset: option reg offset. Enables auto clk mode for entire CMU
> > + * @drcg_offset: dynamic root clk gate enable register offset
> > + * @memclk_offset: memclk enable register offset
>
> dito.

Will fix

Thanks again for your time spent reviewing, some great feedback.

regards,

Peter

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

* Re: [PATCH 7/9] clk: samsung: Implement automatic clock gating mode for CMUs
  2025-10-21 19:45   ` Krzysztof Kozlowski
@ 2025-10-28  9:29     ` Peter Griffin
  0 siblings, 0 replies; 30+ messages in thread
From: Peter Griffin @ 2025-10-28  9:29 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar,
	André Draszik, Tudor Ambarus, Michael Turquette,
	Stephen Boyd, Sam Protsenko, Sylwester Nawrocki, Chanwoo Choi,
	Will McVicker, devicetree, linux-arm-kernel, linux-samsung-soc,
	linux-kernel, linux-clk, Krzysztof Kozlowski, kernel-team

Hi Krzysztof,

Thanks for your review feedback!

On Tue, 21 Oct 2025 at 20:45, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 13/10/2025 22:51, Peter Griffin wrote:
> > Update exynos_arm64_init_clocks() so that it enables the automatic clock
> > mode bits in the CMU option register if the auto_clock_gate flag and
> > option_offset fields are set for the CMU.
> >
> > The CMU option register bits are global and effect every clock component in
> > the CMU, as such clearing the GATE_ENABLE_HWACG bit and setting GATE_MANUAL
> > bit on every gate register is only required if auto_clock_gate is false.
> >
> > Additionally if auto_clock_gate is enabled the dynamic root clock gating
> > and memclk registers will be configured in the corresponding CMUs sysreg
> > bank. These registers are exposed via syscon, so the register
> > suspend/resume paths are updated to handle using a regmap.
> >
> > As many gates for various Samsung SoCs are already exposed in the Samsung
> > clock drivers a new samsung_auto_clk_gate_ops is implemented. This uses
> > some CMU debug registers to report whether clocks are enabled or disabled
> > when operating in automatic mode. This allows
> > /sys/kernel/debug/clk/clk_summary to still dump the entire clock tree and
> > correctly report the status of each clock in the system.
> >
> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > ---
> >  drivers/clk/samsung/clk-exynos-arm64.c   |  47 +++++++--
> >  drivers/clk/samsung/clk-exynos4.c        |   6 +-
> >  drivers/clk/samsung/clk-exynos4412-isp.c |   4 +-
> >  drivers/clk/samsung/clk-exynos5250.c     |   2 +-
> >  drivers/clk/samsung/clk-exynos5420.c     |   4 +-
> >  drivers/clk/samsung/clk.c                | 161 ++++++++++++++++++++++++++++---
> >  drivers/clk/samsung/clk.h                |  49 +++++++++-
> >  7 files changed, 244 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/clk/samsung/clk-exynos-arm64.c b/drivers/clk/samsung/clk-exynos-arm64.c
> > index bf7de21f329ec89069dcf817ca578fcf9b2d9809..c302c836e8f9f6270753d86d7d986c88e6762f4f 100644
> > --- a/drivers/clk/samsung/clk-exynos-arm64.c
> > +++ b/drivers/clk/samsung/clk-exynos-arm64.c
> > @@ -24,6 +24,16 @@
> >  #define GATE_MANUAL          BIT(20)
> >  #define GATE_ENABLE_HWACG    BIT(28)
> >
> > +/* Option register bits */
> > +#define OPT_EN_DBG                   BIT(31)
> > +#define OPT_UNKNOWN                  BIT(30)
> > +#define OPT_EN_PWR_MANAGEMENT                BIT(29)
> > +#define OPT_EN_AUTO_GATING           BIT(28)
> > +#define OPT_EN_MEM_PM_GATING         BIT(24)
>
> Please reverse, from lowest to highest number (LSB -> MSB).
>

Will fix

> > +
> > +#define CMU_OPT_GLOBAL_EN_AUTO_GATING        (OPT_EN_DBG | OPT_UNKNOWN | \
> > +     OPT_EN_PWR_MANAGEMENT | OPT_EN_AUTO_GATING | OPT_EN_MEM_PM_GATING)
> > +
> >  /* PLL_CONx_PLL register offsets range */
> >  #define PLL_CON_OFF_START    0x100
> >  #define PLL_CON_OFF_END              0x600
> > @@ -37,6 +47,8 @@ struct exynos_arm64_cmu_data {
> >       unsigned int nr_clk_save;
> >       const struct samsung_clk_reg_dump *clk_suspend;
> >       unsigned int nr_clk_suspend;
> > +     struct samsung_clk_reg_dump *clk_sysreg_save;
> > +     unsigned int nr_clk_sysreg;
> >
> >       struct clk *clk;
> >       struct clk **pclks;
> > @@ -82,13 +94,28 @@ static void __init exynos_arm64_init_clocks(struct device_node *np,
> >       if (!reg_base)
> >               panic("%s: failed to map registers\n", __func__);
> >
> > +     if (cmu->auto_clock_gate && cmu->option_offset) {
> > +             /*
> > +              * Enable the global automatic mode for the entire CMU.
> > +              * This overrides the individual HWACG bits in each of the
> > +              * individual gate, mux and qch registers.
> > +              */
> > +             writel(CMU_OPT_GLOBAL_EN_AUTO_GATING,
> > +                    reg_base + cmu->option_offset);
> > +     }
> > +
> >       for (i = 0; i < reg_offs_len; ++i) {
> >               void __iomem *reg = reg_base + reg_offs[i];
> >               u32 val;
> >
> >               if (cmu->manual_plls && is_pll_con1_reg(reg_offs[i])) {
> >                       writel(PLL_CON1_MANUAL, reg);
> > -             } else if (is_gate_reg(reg_offs[i])) {
> > +             } else if (is_gate_reg(reg_offs[i]) && !cmu->auto_clock_gate) {
> > +                     /*
> > +                      * Setting GATE_MANUAL bit (which is described in TRM as
> > +                      * reserved!) overrides the global CMU automatic mode
> > +                      * option.
> > +                      */
> >                       val = readl(reg);
> >                       val |= GATE_MANUAL;
> >                       val &= ~GATE_ENABLE_HWACG;
> > @@ -219,7 +246,7 @@ void __init exynos_arm64_register_cmu(struct device *dev,
> >   * Return: 0 on success, or negative error code on error.
> >   */
> >  int __init exynos_arm64_register_cmu_pm(struct platform_device *pdev,
> > -                                     bool set_manual)
> > +                                     bool init_clk_regs)
> >  {
> >       const struct samsung_cmu_info *cmu;
> >       struct device *dev = &pdev->dev;
> > @@ -249,7 +276,7 @@ int __init exynos_arm64_register_cmu_pm(struct platform_device *pdev,
> >               dev_err(dev, "%s: could not enable bus clock %s; err = %d\n",
> >                      __func__, cmu->clk_name, ret);
> >
> > -     if (set_manual)
> > +     if (init_clk_regs)
> >               exynos_arm64_init_clocks(np, cmu);
> >
> >       reg_base = devm_platform_ioremap_resource(pdev, 0);
> > @@ -280,14 +307,18 @@ int exynos_arm64_cmu_suspend(struct device *dev)
> >       struct exynos_arm64_cmu_data *data = dev_get_drvdata(dev);
> >       int i;
> >
> > -     samsung_clk_save(data->ctx->reg_base, data->clk_save,
> > +     samsung_clk_save(data->ctx->reg_base, NULL, data->clk_save,
> >                        data->nr_clk_save);
> >
> > +     if (data->ctx->sysreg)
>
> No need for if() here and in samsung_clk_save.

Will fix.

>
> I find it confusing. You do samsung_clk_save() for regular CMU MMIO
> address and then immediately - for GS101 - do the same for sysreg:
>
> > +             samsung_clk_save(NULL, data->ctx->sysreg, data->clk_save,
> > +                              data->nr_clk_save);
>
> So this feels like you could make one call of samsung_clk_save() with
> both arguments - reg_base and sysreg. It also leads to confusion - first
> read from MMIO will be overwritten by sysreg read.

Apologies for the confusion, that was a typo/bug in v1 it should have
read like this:

samsung_clk_save(data->ctx->reg_base, NULL, data->clk_save,
data->nr_clk_save);

samsung_clk_save(NULL, data->ctx->sysreg, data->clk_sysreg_save,
data->nr_clk_sysreg);

First it saves the CMU MMIO regs, then the second call saves the
sysreg registers (drcg & memclk).

>
>
> > +
> >       for (i = 0; i < data->nr_pclks; i++)
> >               clk_prepare_enable(data->pclks[i]);
> >
> >       /* For suspend some registers have to be set to certain values */
> > -     samsung_clk_restore(data->ctx->reg_base, data->clk_suspend,
> > +     samsung_clk_restore(data->ctx->reg_base, NULL, data->clk_suspend,
> >                           data->nr_clk_suspend);
> >
> >       for (i = 0; i < data->nr_pclks; i++)
> > @@ -308,9 +339,13 @@ int exynos_arm64_cmu_resume(struct device *dev)
> >       for (i = 0; i < data->nr_pclks; i++)
> >               clk_prepare_enable(data->pclks[i]);
> >
> > -     samsung_clk_restore(data->ctx->reg_base, data->clk_save,
> > +     samsung_clk_restore(data->ctx->reg_base, NULL, data->clk_save,
> >                           data->nr_clk_save);
> >
> > +     if (data->ctx->sysreg)
> > +             samsung_clk_restore(NULL, data->ctx->sysreg, data->clk_save,
> > +                                 data->nr_clk_save);
> > +
> >       for (i = 0; i < data->nr_pclks; i++)
> >               clk_disable_unprepare(data->pclks[i]);
> >
> > diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
> > index cc5c1644c41c08b27bc48d809a08cd8a006cbe8f..26ac9734722d1e7ed8ec3f1c0a956f26e32b92d4 100644
> > --- a/drivers/clk/samsung/clk-exynos4.c
> > +++ b/drivers/clk/samsung/clk-exynos4.c
> > @@ -1378,15 +1378,15 @@ static void __init exynos4_clk_init(struct device_node *np,
> >       if (soc == EXYNOS4212 || soc == EXYNOS4412)
> >               exynos4x12_core_down_clock();
> >
> > -     samsung_clk_extended_sleep_init(reg_base,
> > +     samsung_clk_extended_sleep_init(reg_base, NULL,
> >                       exynos4_clk_regs, ARRAY_SIZE(exynos4_clk_regs),
> >                       src_mask_suspend, ARRAY_SIZE(src_mask_suspend));
> >       if (exynos4_soc == EXYNOS4210)
> > -             samsung_clk_extended_sleep_init(reg_base,
> > +             samsung_clk_extended_sleep_init(reg_base, NULL,
> >                   exynos4210_clk_save, ARRAY_SIZE(exynos4210_clk_save),
> >                   src_mask_suspend_e4210, ARRAY_SIZE(src_mask_suspend_e4210));
> >       else
> > -             samsung_clk_sleep_init(reg_base, exynos4x12_clk_save,
> > +             samsung_clk_sleep_init(reg_base, NULL, exynos4x12_clk_save,
> >                                      ARRAY_SIZE(exynos4x12_clk_save));
> >
> >       samsung_clk_of_add_provider(np, ctx);
> > diff --git a/drivers/clk/samsung/clk-exynos4412-isp.c b/drivers/clk/samsung/clk-exynos4412-isp.c
> > index fa915057e109e0008ebe0b1b5d1652fd5804e82b..772bc18a1e686f23b11bf160b803becff6279637 100644
> > --- a/drivers/clk/samsung/clk-exynos4412-isp.c
> > +++ b/drivers/clk/samsung/clk-exynos4412-isp.c
> > @@ -94,7 +94,7 @@ static int __maybe_unused exynos4x12_isp_clk_suspend(struct device *dev)
> >  {
> >       struct samsung_clk_provider *ctx = dev_get_drvdata(dev);
> >
> > -     samsung_clk_save(ctx->reg_base, exynos4x12_save_isp,
> > +     samsung_clk_save(ctx->reg_base, NULL, exynos4x12_save_isp,
> >                        ARRAY_SIZE(exynos4x12_clk_isp_save));
> >       return 0;
> >  }
> > @@ -103,7 +103,7 @@ static int __maybe_unused exynos4x12_isp_clk_resume(struct device *dev)
> >  {
> >       struct samsung_clk_provider *ctx = dev_get_drvdata(dev);
> >
> > -     samsung_clk_restore(ctx->reg_base, exynos4x12_save_isp,
> > +     samsung_clk_restore(ctx->reg_base, NULL, exynos4x12_save_isp,
> >                           ARRAY_SIZE(exynos4x12_clk_isp_save));
> >       return 0;
> >  }
> > diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c
> > index e90d3a0848cbc24b2709c10795f6affcda404567..f97f30b29be7317db8186bac39cf52e1893eb106 100644
> > --- a/drivers/clk/samsung/clk-exynos5250.c
> > +++ b/drivers/clk/samsung/clk-exynos5250.c
> > @@ -854,7 +854,7 @@ static void __init exynos5250_clk_init(struct device_node *np)
> >               PWR_CTRL2_CORE2_UP_RATIO | PWR_CTRL2_CORE1_UP_RATIO);
> >       __raw_writel(tmp, reg_base + PWR_CTRL2);
> >
> > -     samsung_clk_sleep_init(reg_base, exynos5250_clk_regs,
> > +     samsung_clk_sleep_init(reg_base, NULL, exynos5250_clk_regs,
> >                              ARRAY_SIZE(exynos5250_clk_regs));
> >       exynos5_subcmus_init(ctx, ARRAY_SIZE(exynos5250_subcmus),
> >                            exynos5250_subcmus);
> > diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
> > index a9df4e6db82fa7831d4e5c7210b0163d7d301ec1..1982e0751ceec7e57f9e82d96dcbadce1f691092 100644
> > --- a/drivers/clk/samsung/clk-exynos5420.c
> > +++ b/drivers/clk/samsung/clk-exynos5420.c
> > @@ -1649,12 +1649,12 @@ static void __init exynos5x_clk_init(struct device_node *np,
> >                               ARRAY_SIZE(exynos5800_cpu_clks));
> >       }
> >
> > -     samsung_clk_extended_sleep_init(reg_base,
> > +     samsung_clk_extended_sleep_init(reg_base, NULL,
> >               exynos5x_clk_regs, ARRAY_SIZE(exynos5x_clk_regs),
> >               exynos5420_set_clksrc, ARRAY_SIZE(exynos5420_set_clksrc));
> >
> >       if (soc == EXYNOS5800) {
> > -             samsung_clk_sleep_init(reg_base, exynos5800_clk_regs,
> > +             samsung_clk_sleep_init(reg_base, NULL, exynos5800_clk_regs,
> >                                      ARRAY_SIZE(exynos5800_clk_regs));
> >
> >               exynos5_subcmus_init(ctx, ARRAY_SIZE(exynos5800_subcmus),
> > diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c
> > index dbc9925ca8f46e951dfb5d391c0e744ca370abcc..07b2948ae7ea48f126ab420be57d8c2705979464 100644
> > --- a/drivers/clk/samsung/clk.c
> > +++ b/drivers/clk/samsung/clk.c
> > @@ -12,8 +12,10 @@
> >  #include <linux/clkdev.h>
> >  #include <linux/clk-provider.h>
> >  #include <linux/io.h>
> > +#include <linux/mfd/syscon.h>
> >  #include <linux/mod_devicetable.h>
> >  #include <linux/of_address.h>
> > +#include <linux/regmap.h>
> >  #include <linux/syscore_ops.h>
> >
> >  #include "clk.h"
> > @@ -21,19 +23,29 @@
> >  static LIST_HEAD(clock_reg_cache_list);
> >
> >  void samsung_clk_save(void __iomem *base,
> > +                                 struct regmap *regmap,
>
> Here it is regmap, in samsung_clk_extended_sleep_init() it is called
> sysreg.

I'll update samsung_clk_sleep_init so it is named regmap to be
consistent with the naming. Currently it is only used with the sysreg
regmap, but I suppose in the future there could be other regmaps we
wish to save/restore.

>This suggests that in some cases this replaces the MMIO - like
> here in samsung_clk_save().

Each CMU has MMIO registers and nearly all of them (so far only
cmu_top I've found doesn't) have a couple of clock related registers
in the correspondingly named sysreg. This patch was updating the
samsung_clk_save/restore functions so that as well as saving the MMIO
registers via base, you can alternatively pass it a regmap (instead of
base) and it will save those registers as well. It was obfuscated
unfortunately due to the copy/paste bug mentioned above in the first
version.

Maybe we should just have separate functions instead of overloading
these samsung_clk_save_regmap() and samsung_clk_restore_regmap()

>
>
> >                                   struct samsung_clk_reg_dump *rd,
> >                                   unsigned int num_regs)
> >  {
> > -     for (; num_regs > 0; --num_regs, ++rd)
> > -             rd->value = readl(base + rd->offset);
> > +     for (; num_regs > 0; --num_regs, ++rd) {
> > +             if (base)
> > +                     rd->value = readl(base + rd->offset);
> > +             if (regmap)
> > +                     regmap_read(regmap, rd->offset, &rd->value);
> > +     }
> >  }
> >
> >  void samsung_clk_restore(void __iomem *base,
> > +                                   struct regmap *regmap,
> >                                     const struct samsung_clk_reg_dump *rd,
> >                                     unsigned int num_regs)
> >  {
> > -     for (; num_regs > 0; --num_regs, ++rd)
> > -             writel(rd->value, base + rd->offset);
> > +     for (; num_regs > 0; --num_regs, ++rd) {
> > +             if (base)
> > +                     writel(rd->value, base + rd->offset);
> > +             if (regmap)
> > +                     regmap_write(regmap, rd->offset, rd->value);
> > +     }
> >  }
> >
> >  struct samsung_clk_reg_dump *samsung_clk_alloc_reg_dump(
> > @@ -227,6 +239,82 @@ void __init samsung_clk_register_div(struct samsung_clk_provider *ctx,
> >       }
> >  }
> >
> > +#define ACG_MSK GENMASK(6, 4)
> > +#define CLK_IDLE GENMASK(5, 4)
> > +static int samsung_auto_clk_gate_is_en(struct clk_hw *hw)
> > +{
> > +     u32 reg;
> > +     struct clk_gate *gate = to_clk_gate(hw);
> > +
> > +     reg = readl(gate->reg);
> > +     return ((reg & ACG_MSK) == CLK_IDLE) ? 0 : 1;
> > +}
> > +
> > +/* enable and disable are nops in automatic clock mode */
> > +static int samsung_auto_clk_gate_en(struct clk_hw *hw)
> > +{
> > +     return 0;
> > +}
> > +
> > +static void samsung_auto_clk_gate_dis(struct clk_hw *hw)
> > +{
> > +}
> > +
> > +static const struct clk_ops samsung_auto_clk_gate_ops = {
> > +     .enable = samsung_auto_clk_gate_en,
> > +     .disable = samsung_auto_clk_gate_dis,
> > +     .is_enabled = samsung_auto_clk_gate_is_en,
> > +};
> > +
> > +struct clk_hw *samsung_register_auto_gate(struct device *dev,
> > +             struct device_node *np, const char *name,
> > +             const char *parent_name, const struct clk_hw *parent_hw,
> > +             const struct clk_parent_data *parent_data,
> > +             unsigned long flags,
> > +             void __iomem *reg, u8 bit_idx,
> > +             u8 clk_gate_flags, spinlock_t *lock)
> > +{
> > +     struct clk_gate *gate;
> > +     struct clk_hw *hw;
> > +     struct clk_init_data init = {};
> > +     int ret = -EINVAL;
> > +
> > +     /* allocate the gate */
> > +     gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> > +     if (!gate)
> > +             return ERR_PTR(-ENOMEM);
> > +
> > +     init.name = name;
> > +     init.ops = &samsung_auto_clk_gate_ops;
> > +     init.flags = flags;
> > +     init.parent_names = parent_name ? &parent_name : NULL;
> > +     init.parent_hws = parent_hw ? &parent_hw : NULL;
> > +     init.parent_data = parent_data;
> > +     if (parent_name || parent_hw || parent_data)
> > +             init.num_parents = 1;
> > +     else
> > +             init.num_parents = 0;
> > +
> > +     /* struct clk_gate assignments */
> > +     gate->reg = reg;
> > +     gate->bit_idx = bit_idx;
> > +     gate->flags = clk_gate_flags;
> > +     gate->lock = lock;
> > +     gate->hw.init = &init;
> > +
> > +     hw = &gate->hw;
> > +     if (dev || !np)
> > +             ret = clk_hw_register(dev, hw);
> > +     else if (np)
> > +             ret = of_clk_hw_register(np, hw);
> > +     if (ret) {
> > +             kfree(gate);
> > +             hw = ERR_PTR(ret);
> > +     }
> > +
> > +     return hw;
> > +}
> > +
> >  /* register a list of gate clocks */
> >  void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx,
> >                               const struct samsung_gate_clock *list,
> > @@ -234,11 +322,21 @@ void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx,
> >  {
> >       struct clk_hw *clk_hw;
> >       unsigned int idx;
> > +     void __iomem *reg_offs;
> >
> >       for (idx = 0; idx < nr_clk; idx++, list++) {
> > -             clk_hw = clk_hw_register_gate(ctx->dev, list->name, list->parent_name,
> > -                             list->flags, ctx->reg_base + list->offset,
> > +             reg_offs = ctx->reg_base + list->offset;
> > +
> > +             if (ctx->auto_clock_gate && ctx->gate_dbg_offset)
> > +                     clk_hw = samsung_register_auto_gate(ctx->dev, NULL,
> > +                             list->name, list->parent_name, NULL, NULL,
> > +                             list->flags, reg_offs + ctx->gate_dbg_offset,
> >                               list->bit_idx, list->gate_flags, &ctx->lock);
> > +             else
> > +                     clk_hw = clk_hw_register_gate(ctx->dev, list->name,
> > +                             list->parent_name, list->flags,
> > +                             ctx->reg_base + list->offset, list->bit_idx,
> > +                             list->gate_flags, &ctx->lock);
> >               if (IS_ERR(clk_hw)) {
> >                       pr_err("%s: failed to register clock %s\n", __func__,
> >                               list->name);
> > @@ -276,10 +374,11 @@ static int samsung_clk_suspend(void)
> >       struct samsung_clock_reg_cache *reg_cache;
> >
> >       list_for_each_entry(reg_cache, &clock_reg_cache_list, node) {
> > -             samsung_clk_save(reg_cache->reg_base, reg_cache->rdump,
> > -                             reg_cache->rd_num);
> > -             samsung_clk_restore(reg_cache->reg_base, reg_cache->rsuspend,
> > -                             reg_cache->rsuspend_num);
> > +             samsung_clk_save(reg_cache->reg_base, reg_cache->sysreg,
> > +                              reg_cache->rdump, reg_cache->rd_num);
> > +             samsung_clk_restore(reg_cache->reg_base, reg_cache->sysreg,
> > +                                 reg_cache->rsuspend,
> > +                                 reg_cache->rsuspend_num);
> >       }
> >       return 0;
> >  }
> > @@ -289,8 +388,8 @@ static void samsung_clk_resume(void)
> >       struct samsung_clock_reg_cache *reg_cache;
> >
> >       list_for_each_entry(reg_cache, &clock_reg_cache_list, node)
> > -             samsung_clk_restore(reg_cache->reg_base, reg_cache->rdump,
> > -                             reg_cache->rd_num);
> > +             samsung_clk_restore(reg_cache->reg_base, reg_cache->sysreg,
> > +                                 reg_cache->rdump, reg_cache->rd_num);
> >  }
> >
> >  static struct syscore_ops samsung_clk_syscore_ops = {
> > @@ -299,6 +398,7 @@ static struct syscore_ops samsung_clk_syscore_ops = {
> >  };
> >
> >  void samsung_clk_extended_sleep_init(void __iomem *reg_base,
> > +                     struct regmap *sysreg,
> >                       const unsigned long *rdump,
> >                       unsigned long nr_rdump,
> >                       const struct samsung_clk_reg_dump *rsuspend,
> > @@ -319,6 +419,7 @@ void samsung_clk_extended_sleep_init(void __iomem *reg_base,
> >               register_syscore_ops(&samsung_clk_syscore_ops);
> >
> >       reg_cache->reg_base = reg_base;
> > +     reg_cache->sysreg = sysreg;
> >       reg_cache->rd_num = nr_rdump;
> >       reg_cache->rsuspend = rsuspend;
> >       reg_cache->rsuspend_num = nr_rsuspend;
> > @@ -334,6 +435,12 @@ void samsung_clk_extended_sleep_init(void __iomem *reg_base,
> >  void __init samsung_cmu_register_clocks(struct samsung_clk_provider *ctx,
> >                                       const struct samsung_cmu_info *cmu)
> >  {
> > +     ctx->auto_clock_gate = cmu->auto_clock_gate;
> > +     ctx->gate_dbg_offset = cmu->gate_dbg_offset;
> > +     ctx->option_offset = cmu->option_offset;
> > +     ctx->drcg_offset = cmu->drcg_offset;
> > +     ctx->memclk_offset = cmu->memclk_offset;
> > +
> >       if (cmu->pll_clks)
> >               samsung_clk_register_pll(ctx, cmu->pll_clks, cmu->nr_pll_clks);
> >       if (cmu->mux_clks)
> > @@ -353,6 +460,31 @@ void __init samsung_cmu_register_clocks(struct samsung_clk_provider *ctx,
> >               samsung_clk_register_cpu(ctx, cmu->cpu_clks, cmu->nr_cpu_clks);
> >  }
> >
> > +/* Enable Dynamic Root Clock Gating of bus components*/
> > +void samsung_en_dyn_root_clk_gating(struct device_node *np,
> > +                                 struct samsung_clk_provider *ctx,
> > +                                 const struct samsung_cmu_info *cmu)
> > +{
> > +     if (ctx && !ctx->auto_clock_gate)
>
> Test for ctx should trigger Smatch warning.

Will remove the ctx test.
>
> > +             return;
> > +
> > +     ctx->sysreg = syscon_regmap_lookup_by_phandle(np, "samsung,sysreg");
> > +     if (!IS_ERR_OR_NULL(ctx->sysreg)) {
>
> Can it return NULL? anyway that's very confusing if() - negated OR_NULL.
> Better just
> if (IS_ERR()) {
>         ctx->sysreg = NULL;
>         return;
> }

Will update like you suggest in v2. Thanks for the review feedback.

regards,

Peter

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

end of thread, other threads:[~2025-10-28  9:29 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-13 20:51 [PATCH 0/9] Implement hardware automatic clock gating (HWACG) for gs101 Peter Griffin
2025-10-13 20:51 ` [PATCH 1/9] dt-bindings: soc: samsung: exynos-sysreg: add gs101 hsi0 and misc compatibles Peter Griffin
2025-10-15 18:31   ` Rob Herring (Arm)
2025-10-16  5:44   ` André Draszik
2025-10-21 19:27   ` (subset) " Krzysztof Kozlowski
2025-10-13 20:51 ` [PATCH 2/9] dt-bindings: clock: google,gs101-clock: add samsung,sysreg property as required Peter Griffin
2025-10-15 18:32   ` Rob Herring (Arm)
2025-10-16  5:53   ` André Draszik
2025-10-21 19:22   ` Krzysztof Kozlowski
2025-10-13 20:51 ` [PATCH 3/9] arm64: dts: exynos: gs101: add sysreg_misc and sysreg_hsi0 nodes Peter Griffin
2025-10-16  5:56   ` André Draszik
2025-10-21 19:27   ` (subset) " Krzysztof Kozlowski
2025-10-13 20:51 ` [PATCH 4/9] arm64: dts: exynos: gs101: fix clock module unit reg sizes Peter Griffin
2025-10-16  8:22   ` André Draszik
2025-10-21 19:27   ` (subset) " Krzysztof Kozlowski
2025-10-13 20:51 ` [PATCH 5/9] arm64: dts: exynos: gs101: fix sysreg_apm reg property Peter Griffin
2025-10-16  8:23   ` André Draszik
2025-10-21 19:27   ` (subset) " Krzysztof Kozlowski
2025-10-13 20:51 ` [PATCH 6/9] arm64: dts: exynos: gs101: add samsung,sysreg property to CMU nodes Peter Griffin
2025-10-13 20:51 ` [PATCH 7/9] clk: samsung: Implement automatic clock gating mode for CMUs Peter Griffin
2025-10-14 14:46   ` kernel test robot
2025-10-16  9:40   ` André Draszik
2025-10-28  8:50     ` Peter Griffin
2025-10-21 19:45   ` Krzysztof Kozlowski
2025-10-28  9:29     ` Peter Griffin
2025-10-13 20:51 ` [PATCH 8/9] clk: samsung: gs101: Enable auto_clock_gate mode for each gs101 CMU Peter Griffin
2025-10-21 19:47   ` Krzysztof Kozlowski
2025-10-21 21:03     ` Peter Griffin
2025-10-22  5:45       ` Krzysztof Kozlowski
2025-10-13 20:51 ` [PATCH 9/9] clk: samsung: gs101: remove CLK_IGNORE_UNUSED and CLK_IS_CRITICAL flags Peter Griffin

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