[PATCH v2 00/10] clk: renesas: rzg2l: Add support for power domains

[PATCH v2 00/10] clk: renesas: rzg2l: Add support for power domains

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Hi,

Series adds support for power domains on rzg2l driver.

RZ/G2L kind of devices support a functionality called MSTOP (module
stop/standby). According to hardware manual the module could be switch
to standby after its clocks are disabled. The reverse order of operation
should be done when enabling a module (get the module out of standby,
enable its clocks etc).

In [1] the MSTOP settings were implemented by adding code in driver
to attach the MSTOP state to the IP clocks. But it has been proposed
to implement it as power domain. The result is this series.

Along with MSTOP functionality there is also module power down
functionality (which is currently available only on RZ/G3S). This has
been also implemented through power domains.

The DT bindings were updated with power domain IDs (plain integers
that matches the DT with driver data structures). The current DT
bindings were updated with module IDs for the modules listed in tables
with name "Registers for Module Standby Mode" (see HW manual) exception
being RZ/G3S where, due to the power down functionality, the DDR,
TZCDDR, OTFDE_DDR were also added, to avoid system being blocked due
to the following lines of code from patch 7/17.

+       /* Prepare for power down the BUSes in power down mode. */
+       if (info->pm_domain_pwrdn_mstop)
+               writel(CPG_PWRDN_MSTOP_ENABLE, priv->base + CPG_PWRDN_MSTOP);

Domain IDs were added to all SoC specific bindings.

Thank you,
Claudiu Beznea 

Changes in v2:
- addressed review comments
- dropped:
    - dt-bindings: clock: r9a09g011-cpg: Add always-on power domain IDs
    - clk: renesas: r9a07g043: Add initial support for power domains
    - clk: renesas: r9a07g044: Add initial support for power domains
    - clk: renesas: r9a09g011: Add initial support for power domains
    - clk: renesas: r9a09g011: Add initial support for power domains
    - arm64: dts: renesas: r9a07g043: Update #power-domain-cells = <1>
    - arm64: dts: renesas: r9a07g044: Update #power-domain-cells = <1>
    - arm64: dts: renesas: r9a07g054: Update #power-domain-cells = <1>
    - arm64: dts: renesas: r9a09g011: Update #power-domain-cells = <1>
  as suggested in the review process
- dropped "arm64: dts: renesas: rzg3s-smarc-som: Guard the ethernet IRQ
  GPIOs with proper flags" patch as it was integrated
- added suspend to RAM support
- collected tag

[1] https://lore.kernel.org/all/20231120070024.4079344-4-claudiu.beznea.uj@bp.renesas.com/

Claudiu Beznea (10):
  dt-bindings: clock: r9a07g043-cpg: Add power domain IDs
  dt-bindings: clock: r9a07g044-cpg: Add power domain IDs
  dt-bindings: clock: r9a07g054-cpg: Add power domain IDs
  dt-bindings: clock: r9a08g045-cpg: Add power domain IDs
  dt-bindings: clock: renesas,rzg2l-cpg: Update #power-domain-cells =
    <1> for RZ/G3S
  clk: renesas: rzg2l: Extend power domain support
  clk: renesas: r9a08g045: Add support for power domains
  clk: renesas: rzg2l-cpg: Add suspend/resume support for power domains
  clk: renesas: r9a08g045: Add the RZG2L_PD_F_CONSOLE flag to scif0 PM
    domain
  arm64: dts: renesas: r9a08g045: Update #power-domain-cells = <1>

 .../bindings/clock/renesas,rzg2l-cpg.yaml     |  18 +-
 arch/arm64/boot/dts/renesas/r9a08g045.dtsi    |  20 +-
 drivers/clk/renesas/r9a08g045-cpg.c           |  61 ++++
 drivers/clk/renesas/rzg2l-cpg.c               | 267 +++++++++++++++++-
 drivers/clk/renesas/rzg2l-cpg.h               |  78 +++++
 include/dt-bindings/clock/r9a07g043-cpg.h     |  52 ++++
 include/dt-bindings/clock/r9a07g044-cpg.h     |  58 ++++
 include/dt-bindings/clock/r9a07g054-cpg.h     |  58 ++++
 include/dt-bindings/clock/r9a08g045-cpg.h     |  70 +++++
 9 files changed, 658 insertions(+), 24 deletions(-)

-- 
2.39.2

[PATCH v2 01/10] dt-bindings: clock: r9a07g043-cpg: Add power domain IDs

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Add power domain IDs for RZ/G2UL (R9A07G043) SoC.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v2:
- added "RZ/G2UL Only" comments to some defines
- added RZ/Five specific defines

 include/dt-bindings/clock/r9a07g043-cpg.h | 52 +++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/include/dt-bindings/clock/r9a07g043-cpg.h b/include/dt-bindings/clock/r9a07g043-cpg.h
index 77cde8effdc7..ba9ea276c4a0 100644
--- a/include/dt-bindings/clock/r9a07g043-cpg.h
+++ b/include/dt-bindings/clock/r9a07g043-cpg.h
@@ -200,5 +200,57 @@
 #define R9A07G043_AX45MP_CORE0_RESETN	78	/* RZ/Five Only */
 #define R9A07G043_IAX45_RESETN		79	/* RZ/Five Only */
 
+/* Power domain IDs. */
+#define R9A07G043_PD_ALWAYS_ON		0
+#define R9A07G043_PD_GIC		1	/* RZ/G2UL Only */
+#define R9A07G043_PD_IA55		2	/* RZ/G2UL Only */
+#define R9A07G043_PD_MHU		3	/* RZ/G2UL Only */
+#define R9A07G043_PD_CORESIGHT		4	/* RZ/G2UL Only */
+#define R9A07G043_PD_SYC		5	/* RZ/G2UL Only */
+#define R9A07G043_PD_DMAC		6
+#define R9A07G043_PD_GTM0		7
+#define R9A07G043_PD_GTM1		8
+#define R9A07G043_PD_GTM2		9
+#define R9A07G043_PD_MTU		10
+#define R9A07G043_PD_POE3		11
+#define R9A07G043_PD_WDT0		12
+#define R9A07G043_PD_SPI		13
+#define R9A07G043_PD_SDHI0		14
+#define R9A07G043_PD_SDHI1		15
+#define R9A07G043_PD_ISU		16	/* RZ/G2UL Only */
+#define R9A07G043_PD_CRU		17	/* RZ/G2UL Only */
+#define R9A07G043_PD_LCDC		18	/* RZ/G2UL Only */
+#define R9A07G043_PD_SSI0		19
+#define R9A07G043_PD_SSI1		20
+#define R9A07G043_PD_SSI2		21
+#define R9A07G043_PD_SSI3		22
+#define R9A07G043_PD_SRC		23
+#define R9A07G043_PD_USB0		24
+#define R9A07G043_PD_USB1		25
+#define R9A07G043_PD_USB_PHY		26
+#define R9A07G043_PD_ETHER0		27
+#define R9A07G043_PD_ETHER1		28
+#define R9A07G043_PD_I2C0		29
+#define R9A07G043_PD_I2C1		30
+#define R9A07G043_PD_I2C2		31
+#define R9A07G043_PD_I2C3		32
+#define R9A07G043_PD_SCIF0		33
+#define R9A07G043_PD_SCIF1		34
+#define R9A07G043_PD_SCIF2		35
+#define R9A07G043_PD_SCIF3		36
+#define R9A07G043_PD_SCIF4		37
+#define R9A07G043_PD_SCI0		38
+#define R9A07G043_PD_SCI1		39
+#define R9A07G043_PD_IRDA		40
+#define R9A07G043_PD_RSPI0		41
+#define R9A07G043_PD_RSPI1		42
+#define R9A07G043_PD_RSPI2		43
+#define R9A07G043_PD_CANFD		44
+#define R9A07G043_PD_ADC		45
+#define R9A07G043_PD_TSU		46
+#define R9A07G043_PD_PLIC		47	/* RZ/Five Only */
+#define R9A07G043_PD_IAX45		48	/* RZ/Five Only */
+#define R9A07G043_PD_NCEPLDM		49	/* RZ/Five Only */
+#define R9A07G043_PD_NCEPLMT		50	/* RZ/Five Only */
 
 #endif /* __DT_BINDINGS_CLOCK_R9A07G043_CPG_H__ */
-- 
2.39.2

[PATCH v2 02/10] dt-bindings: clock: r9a07g044-cpg: Add power domain IDs

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Add power domain IDs for RZ/G2L (R9A07G044) SoC.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---

Changes in v2:
- collected tag

 include/dt-bindings/clock/r9a07g044-cpg.h | 58 +++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/include/dt-bindings/clock/r9a07g044-cpg.h b/include/dt-bindings/clock/r9a07g044-cpg.h
index 0bb17ff1a01a..e209f96f92b7 100644
--- a/include/dt-bindings/clock/r9a07g044-cpg.h
+++ b/include/dt-bindings/clock/r9a07g044-cpg.h
@@ -217,4 +217,62 @@
 #define R9A07G044_ADC_ADRST_N		82
 #define R9A07G044_TSU_PRESETN		83
 
+/* Power domain IDs. */
+#define R9A07G044_PD_ALWAYS_ON		0
+#define R9A07G044_PD_GIC		1
+#define R9A07G044_PD_IA55		2
+#define R9A07G044_PD_MHU		3
+#define R9A07G044_PD_CORESIGHT		4
+#define R9A07G044_PD_SYC		5
+#define R9A07G044_PD_DMAC		6
+#define R9A07G044_PD_GTM0		7
+#define R9A07G044_PD_GTM1		8
+#define R9A07G044_PD_GTM2		9
+#define R9A07G044_PD_MTU		10
+#define R9A07G044_PD_POE3		11
+#define R9A07G044_PD_GPT		12
+#define R9A07G044_PD_POEGA		13
+#define R9A07G044_PD_POEGB		14
+#define R9A07G044_PD_POEGC		15
+#define R9A07G044_PD_POEGD		16
+#define R9A07G044_PD_WDT0		17
+#define R9A07G044_PD_WDT1		18
+#define R9A07G044_PD_SPI		19
+#define R9A07G044_PD_SDHI0		20
+#define R9A07G044_PD_SDHI1		21
+#define R9A07G044_PD_3DGE		22
+#define R9A07G044_PD_ISU		23
+#define R9A07G044_PD_VCPL4		24
+#define R9A07G044_PD_CRU		25
+#define R9A07G044_PD_MIPI_DSI		26
+#define R9A07G044_PD_LCDC		27
+#define R9A07G044_PD_SSI0		28
+#define R9A07G044_PD_SSI1		29
+#define R9A07G044_PD_SSI2		30
+#define R9A07G044_PD_SSI3		31
+#define R9A07G044_PD_SRC		32
+#define R9A07G044_PD_USB0		33
+#define R9A07G044_PD_USB1		34
+#define R9A07G044_PD_USB_PHY		35
+#define R9A07G044_PD_ETHER0		36
+#define R9A07G044_PD_ETHER1		37
+#define R9A07G044_PD_I2C0		38
+#define R9A07G044_PD_I2C1		39
+#define R9A07G044_PD_I2C2		40
+#define R9A07G044_PD_I2C3		41
+#define R9A07G044_PD_SCIF0		42
+#define R9A07G044_PD_SCIF1		43
+#define R9A07G044_PD_SCIF2		44
+#define R9A07G044_PD_SCIF3		45
+#define R9A07G044_PD_SCIF4		46
+#define R9A07G044_PD_SCI0		47
+#define R9A07G044_PD_SCI1		48
+#define R9A07G044_PD_IRDA		49
+#define R9A07G044_PD_RSPI0		50
+#define R9A07G044_PD_RSPI1		51
+#define R9A07G044_PD_RSPI2		52
+#define R9A07G044_PD_CANFD		53
+#define R9A07G044_PD_ADC		54
+#define R9A07G044_PD_TSU		55
+
 #endif /* __DT_BINDINGS_CLOCK_R9A07G044_CPG_H__ */
-- 
2.39.2

[PATCH v2 03/10] dt-bindings: clock: r9a07g054-cpg: Add power domain IDs

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Add power domain IDs for RZ/V2L (R9A07G054) SoC.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---

Changes in v2:
- collected tag

 include/dt-bindings/clock/r9a07g054-cpg.h | 58 +++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/include/dt-bindings/clock/r9a07g054-cpg.h b/include/dt-bindings/clock/r9a07g054-cpg.h
index 43f4dbda872c..2c99f89397c4 100644
--- a/include/dt-bindings/clock/r9a07g054-cpg.h
+++ b/include/dt-bindings/clock/r9a07g054-cpg.h
@@ -226,4 +226,62 @@
 #define R9A07G054_TSU_PRESETN		83
 #define R9A07G054_STPAI_ARESETN		84
 
+/* Power domain IDs. */
+#define R9A07G054_PD_ALWAYS_ON		0
+#define R9A07G054_PD_GIC		1
+#define R9A07G054_PD_IA55		2
+#define R9A07G054_PD_MHU		3
+#define R9A07G054_PD_CORESIGHT		4
+#define R9A07G054_PD_SYC		5
+#define R9A07G054_PD_DMAC		6
+#define R9A07G054_PD_GTM0		7
+#define R9A07G054_PD_GTM1		8
+#define R9A07G054_PD_GTM2		9
+#define R9A07G054_PD_MTU		10
+#define R9A07G054_PD_POE3		11
+#define R9A07G054_PD_GPT		12
+#define R9A07G054_PD_POEGA		13
+#define R9A07G054_PD_POEGB		14
+#define R9A07G054_PD_POEGC		15
+#define R9A07G054_PD_POEGD		16
+#define R9A07G054_PD_WDT0		17
+#define R9A07G054_PD_WDT1		18
+#define R9A07G054_PD_SPI		19
+#define R9A07G054_PD_SDHI0		20
+#define R9A07G054_PD_SDHI1		21
+#define R9A07G054_PD_3DGE		22
+#define R9A07G054_PD_ISU		23
+#define R9A07G054_PD_VCPL4		24
+#define R9A07G054_PD_CRU		25
+#define R9A07G054_PD_MIPI_DSI		26
+#define R9A07G054_PD_LCDC		27
+#define R9A07G054_PD_SSI0		28
+#define R9A07G054_PD_SSI1		29
+#define R9A07G054_PD_SSI2		30
+#define R9A07G054_PD_SSI3		31
+#define R9A07G054_PD_SRC		32
+#define R9A07G054_PD_USB0		33
+#define R9A07G054_PD_USB1		34
+#define R9A07G054_PD_USB_PHY		35
+#define R9A07G054_PD_ETHER0		36
+#define R9A07G054_PD_ETHER1		37
+#define R9A07G054_PD_I2C0		38
+#define R9A07G054_PD_I2C1		39
+#define R9A07G054_PD_I2C2		40
+#define R9A07G054_PD_I2C3		41
+#define R9A07G054_PD_SCIF0		42
+#define R9A07G054_PD_SCIF1		43
+#define R9A07G054_PD_SCIF2		44
+#define R9A07G054_PD_SCIF3		45
+#define R9A07G054_PD_SCIF4		46
+#define R9A07G054_PD_SCI0		47
+#define R9A07G054_PD_SCI1		48
+#define R9A07G054_PD_IRDA		49
+#define R9A07G054_PD_RSPI0		50
+#define R9A07G054_PD_RSPI1		51
+#define R9A07G054_PD_RSPI2		52
+#define R9A07G054_PD_CANFD		53
+#define R9A07G054_PD_ADC		54
+#define R9A07G054_PD_TSU		55
+
 #endif /* __DT_BINDINGS_CLOCK_R9A07G054_CPG_H__ */
-- 
2.39.2

[PATCH v2 04/10] dt-bindings: clock: r9a08g045-cpg: Add power domain IDs

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Add power domain IDs for RZ/G3S (R9A08G045) SoC.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---

Changes in v2:
- collected tag

 include/dt-bindings/clock/r9a08g045-cpg.h | 70 +++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/include/dt-bindings/clock/r9a08g045-cpg.h b/include/dt-bindings/clock/r9a08g045-cpg.h
index 410725b778a8..8281e9caf3a9 100644
--- a/include/dt-bindings/clock/r9a08g045-cpg.h
+++ b/include/dt-bindings/clock/r9a08g045-cpg.h
@@ -239,4 +239,74 @@
 #define R9A08G045_I3C_PRESETN		92
 #define R9A08G045_VBAT_BRESETN		93
 
+/* Power domain IDs. */
+#define R9A08G045_PD_ALWAYS_ON		0
+#define R9A08G045_PD_GIC		1
+#define R9A08G045_PD_IA55		2
+#define R9A08G045_PD_MHU		3
+#define R9A08G045_PD_CORESIGHT		4
+#define R9A08G045_PD_SYC		5
+#define R9A08G045_PD_DMAC		6
+#define R9A08G045_PD_GTM0		7
+#define R9A08G045_PD_GTM1		8
+#define R9A08G045_PD_GTM2		9
+#define R9A08G045_PD_GTM3		10
+#define R9A08G045_PD_GTM4		11
+#define R9A08G045_PD_GTM5		12
+#define R9A08G045_PD_GTM6		13
+#define R9A08G045_PD_GTM7		14
+#define R9A08G045_PD_MTU		15
+#define R9A08G045_PD_POE3		16
+#define R9A08G045_PD_GPT		17
+#define R9A08G045_PD_POEGA		18
+#define R9A08G045_PD_POEGB		19
+#define R9A08G045_PD_POEGC		20
+#define R9A08G045_PD_POEGD		21
+#define R9A08G045_PD_WDT0		22
+#define R9A08G045_PD_XSPI		23
+#define R9A08G045_PD_SDHI0		24
+#define R9A08G045_PD_SDHI1		25
+#define R9A08G045_PD_SDHI2		26
+#define R9A08G045_PD_SSI0		27
+#define R9A08G045_PD_SSI1		28
+#define R9A08G045_PD_SSI2		29
+#define R9A08G045_PD_SSI3		30
+#define R9A08G045_PD_SRC		31
+#define R9A08G045_PD_USB0		32
+#define R9A08G045_PD_USB1		33
+#define R9A08G045_PD_USB_PHY		34
+#define R9A08G045_PD_ETHER0		35
+#define R9A08G045_PD_ETHER1		36
+#define R9A08G045_PD_I2C0		37
+#define R9A08G045_PD_I2C1		38
+#define R9A08G045_PD_I2C2		39
+#define R9A08G045_PD_I2C3		40
+#define R9A08G045_PD_SCIF0		41
+#define R9A08G045_PD_SCIF1		42
+#define R9A08G045_PD_SCIF2		43
+#define R9A08G045_PD_SCIF3		44
+#define R9A08G045_PD_SCIF4		45
+#define R9A08G045_PD_SCIF5		46
+#define R9A08G045_PD_SCI0		47
+#define R9A08G045_PD_SCI1		48
+#define R9A08G045_PD_IRDA		49
+#define R9A08G045_PD_RSPI0		50
+#define R9A08G045_PD_RSPI1		51
+#define R9A08G045_PD_RSPI2		52
+#define R9A08G045_PD_RSPI3		53
+#define R9A08G045_PD_RSPI4		54
+#define R9A08G045_PD_CANFD		55
+#define R9A08G045_PD_ADC		56
+#define R9A08G045_PD_TSU		57
+#define R9A08G045_PD_OCTA		58
+#define R9A08G045_PD_PDM		59
+#define R9A08G045_PD_PCI		60
+#define R9A08G045_PD_SPDIF		61
+#define R9A08G045_PD_I3C		62
+#define R9A08G045_PD_VBAT		63
+
+#define R9A08G045_PD_DDR		64
+#define R9A08G045_PD_TZCDDR		65
+#define R9A08G045_PD_OTFDE_DDR		66
+
 #endif /* __DT_BINDINGS_CLOCK_R9A08G045_CPG_H__ */
-- 
2.39.2

[PATCH v2 05/10] dt-bindings: clock: renesas,rzg2l-cpg: Update #power-domain-cells = <1> for RZ/G3S

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

The driver will be modified (in the next commits) to be able to specify
individual power domain ID for each IP. The driver will still
support #power-domain-cells = <0>, thus, previous users are not
affected.

The #power-domain-cells = <1> has been instantiated only for RZ/G3S at
the moment as individual platform clock drivers need to be adapted for
this to be supported on the rest of the SoCs.

Also, the description for #power-domain-cells was updated with the links
to per-SoC power domain IDs.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v2:
- updated patch title and description
- kept both 0 and 1 for #power-domain-cells as not all the drivers,
  device trees are adpated with this series
- added a reference to dt-bindings/clock/r9a0*-cpg.h for power domain
  specifiers
- dropped the changes from examples section

 .../bindings/clock/renesas,rzg2l-cpg.yaml      | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/clock/renesas,rzg2l-cpg.yaml b/Documentation/devicetree/bindings/clock/renesas,rzg2l-cpg.yaml
index 80a8c7114c31..4e3b0c45124a 100644
--- a/Documentation/devicetree/bindings/clock/renesas,rzg2l-cpg.yaml
+++ b/Documentation/devicetree/bindings/clock/renesas,rzg2l-cpg.yaml
@@ -57,7 +57,8 @@ properties:
       can be power-managed through Module Standby should refer to the CPG device
       node in their "power-domains" property, as documented by the generic PM
       Domain bindings in Documentation/devicetree/bindings/power/power-domain.yaml.
-    const: 0
+      The power domain specifiers defined in <dt-bindings/clock/r9a0*-cpg.h> could
+      be used to reference individual CPG power domains.
 
   '#reset-cells':
     description:
@@ -76,6 +77,21 @@ required:
 
 additionalProperties: false
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: renesas,r9a08g045-cpg
+    then:
+      properties:
+        '#power-domain-cells':
+          const: 1
+    else:
+      properties:
+        '#power-domain-cells':
+          const: 0
+
 examples:
   - |
     cpg: clock-controller@11010000 {
-- 
2.39.2

[PATCH v2 06/10] clk: renesas: rzg2l: Extend power domain support

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

RZ/{G2L, V2L, G3S}-based CPG versions have support for saving extra
power when clocks are disabled by activating module standby. This is done
through MSTOP-specific registers that are part of CPG. Each individual
module has one or more bits associated with one MSTOP register (see table
"Registers for Module Standby Mode" from HW manuals). Hardware manual
associates modules' clocks with one or more MSTOP bits. There are 3
mappings available (identified by researching RZ/G2L, RZ/G3S, RZ/V2L HW
manuals):

case 1: N clocks mapped to N MSTOP bits (with N={0, ..., X})
case 2: N clocks mapped to 1 MSTOP bit  (with N={0, ..., X})
case 3: N clocks mapped to M MSTOP bits (with N={0, ..., X}, M={0, ..., Y})

Case 3 has been currently identified on RZ/V2L for the VCPL4 module.

To cover all three cases, the individual platform drivers will provide to
clock driver MSTOP register offset and associated bits in this register
as a bitmask and the clock driver will apply this bitmask to proper
MSTOP register.

Apart from MSTOP support, RZ/G3S can save more power by powering down the
individual IPs (after MSTOP has been set) if proper bits in
CPG_PWRDN_IP{1,2} registers are set.

The MSTOP and IP power down support were implemented through power
domains. Platform-specific clock drivers will register an array of
type struct rzg2l_cpg_pm_domain_init_data, which will be used to
instantiate properly the power domains.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v2:
- called pm_genpd_init() with proper value for is_off argument
- fixed typos
- used flexible array for struct rzg2l_cpg_pm_domains::domains member
- moved genpd member of struct rzg2l_cpg_pd at the beginning of struct
- didn't initialize the parent variable in rzg2l_cpg_add_pm_domains()
  as it is already initialized in the for block from
  rzg2l_cpg_add_pm_domains() and that initialization should be enough
- dropped RZG2L_PD_F_PARENT flag
- used datasheet naming for all MSTOP registers
- added all MSTOP registers to rzg2l-cpg.h
- reworked the code that initializes the register offset and bits for domains
- dropped MSTOP*(), PWRDN*() macros and introduced struct rzg2l_cpg_reg_conf
  and DEF_REG_CONF() for domain description
- constified the 1st argument of rzg2l_cpg_pm_domain_xlate()
- used dev instead of priv->dev where possible
- dropped RZG2L_PD_F_PARENT
- added RZG2L_PD_F_NONE for better description of domains in platform
  specific clock drivers

 drivers/clk/renesas/rzg2l-cpg.c | 213 +++++++++++++++++++++++++++++---
 drivers/clk/renesas/rzg2l-cpg.h |  77 ++++++++++++
 2 files changed, 276 insertions(+), 14 deletions(-)

diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c
index 3d2daa4ba2a4..b36700f4a9f5 100644
--- a/drivers/clk/renesas/rzg2l-cpg.c
+++ b/drivers/clk/renesas/rzg2l-cpg.c
@@ -139,7 +139,6 @@ struct rzg2l_pll5_mux_dsi_div_param {
  * @num_resets: Number of Module Resets in info->resets[]
  * @last_dt_core_clk: ID of the last Core Clock exported to DT
  * @info: Pointer to platform data
- * @genpd: PM domain
  * @mux_dsi_div_params: pll5 mux and dsi div parameters
  */
 struct rzg2l_cpg_priv {
@@ -156,8 +155,6 @@ struct rzg2l_cpg_priv {
 
 	const struct rzg2l_cpg_info *info;
 
-	struct generic_pm_domain genpd;
-
 	struct rzg2l_pll5_mux_dsi_div_param mux_dsi_div_params;
 };
 
@@ -1559,9 +1556,34 @@ static bool rzg2l_cpg_is_pm_clk(struct rzg2l_cpg_priv *priv,
 	return true;
 }
 
+/**
+ * struct rzg2l_cpg_pm_domains - RZ/G2L PM domains data structure
+ * @onecell_data: cell data
+ * @domains: generic PM domains
+ */
+struct rzg2l_cpg_pm_domains {
+	struct genpd_onecell_data onecell_data;
+	struct generic_pm_domain *domains[];
+};
+
+/**
+ * struct rzg2l_cpg_pd - RZ/G2L power domain data structure
+ * @genpd: generic PM domain
+ * @priv: pointer to CPG private data structure
+ * @conf: CPG PM domain configuration info
+ * @id: RZ/G2L power domain ID
+ */
+struct rzg2l_cpg_pd {
+	struct generic_pm_domain genpd;
+	struct rzg2l_cpg_priv *priv;
+	struct rzg2l_cpg_pm_domain_conf conf;
+	u16 id;
+};
+
 static int rzg2l_cpg_attach_dev(struct generic_pm_domain *domain, struct device *dev)
 {
-	struct rzg2l_cpg_priv *priv = container_of(domain, struct rzg2l_cpg_priv, genpd);
+	struct rzg2l_cpg_pd *pd = container_of(domain, struct rzg2l_cpg_pd, genpd);
+	struct rzg2l_cpg_priv *priv = pd->priv;
 	struct device_node *np = dev->of_node;
 	struct of_phandle_args clkspec;
 	bool once = true;
@@ -1617,31 +1639,194 @@ static void rzg2l_cpg_detach_dev(struct generic_pm_domain *unused, struct device
 }
 
 static void rzg2l_cpg_genpd_remove(void *data)
+{
+	struct genpd_onecell_data *celldata = data;
+
+	for (unsigned int i = 0; i < celldata->num_domains; i++)
+		pm_genpd_remove(celldata->domains[i]);
+}
+
+static void rzg2l_cpg_genpd_remove_simple(void *data)
 {
 	pm_genpd_remove(data);
 }
 
+static int rzg2l_cpg_power_on(struct generic_pm_domain *domain)
+{
+	struct rzg2l_cpg_pd *pd = container_of(domain, struct rzg2l_cpg_pd, genpd);
+	struct rzg2l_cpg_reg_conf mstop = pd->conf.mstop;
+	struct rzg2l_cpg_reg_conf pwrdn = pd->conf.pwrdn;
+	struct rzg2l_cpg_priv *priv = pd->priv;
+
+	/* Set PWRDN. */
+	if (pwrdn.mask)
+		writel(pwrdn.mask << 16, priv->base + pwrdn.off);
+
+	/* Set MSTOP. */
+	if (mstop.mask)
+		writel(mstop.mask << 16, priv->base + mstop.off);
+
+	return 0;
+}
+
+static int rzg2l_cpg_power_off(struct generic_pm_domain *domain)
+{
+	struct rzg2l_cpg_pd *pd = container_of(domain, struct rzg2l_cpg_pd, genpd);
+	struct rzg2l_cpg_reg_conf mstop = pd->conf.mstop;
+	struct rzg2l_cpg_reg_conf pwrdn = pd->conf.pwrdn;
+	struct rzg2l_cpg_priv *priv = pd->priv;
+
+	/* Set MSTOP. */
+	if (mstop.mask)
+		writel(mstop.mask | (mstop.mask << 16), priv->base + mstop.off);
+
+	/* Set PWRDN. */
+	if (pwrdn.mask)
+		writel(pwrdn.mask | (pwrdn.mask << 16), priv->base + pwrdn.off);
+
+	return 0;
+}
+
+static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, bool always_on)
+{
+	struct dev_power_governor *governor;
+
+	pd->genpd.flags |= GENPD_FLAG_PM_CLK | GENPD_FLAG_ACTIVE_WAKEUP;
+	pd->genpd.attach_dev = rzg2l_cpg_attach_dev;
+	pd->genpd.detach_dev = rzg2l_cpg_detach_dev;
+	if (always_on) {
+		pd->genpd.flags |= GENPD_FLAG_ALWAYS_ON;
+		governor = &pm_domain_always_on_gov;
+	} else {
+		pd->genpd.power_on = rzg2l_cpg_power_on;
+		pd->genpd.power_off = rzg2l_cpg_power_off;
+		governor = &simple_qos_governor;
+	}
+
+	return pm_genpd_init(&pd->genpd, governor, !always_on);
+}
+
 static int __init rzg2l_cpg_add_clk_domain(struct rzg2l_cpg_priv *priv)
 {
 	struct device *dev = priv->dev;
 	struct device_node *np = dev->of_node;
-	struct generic_pm_domain *genpd = &priv->genpd;
+	struct rzg2l_cpg_pd *pd;
 	int ret;
 
-	genpd->name = np->name;
-	genpd->flags = GENPD_FLAG_PM_CLK | GENPD_FLAG_ALWAYS_ON |
-		       GENPD_FLAG_ACTIVE_WAKEUP;
-	genpd->attach_dev = rzg2l_cpg_attach_dev;
-	genpd->detach_dev = rzg2l_cpg_detach_dev;
-	ret = pm_genpd_init(genpd, &pm_domain_always_on_gov, false);
+	pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
+	if (!pd)
+		return -ENOMEM;
+
+	pd->genpd.name = np->name;
+	pd->priv = priv;
+	ret = rzg2l_cpg_pd_setup(pd, true);
 	if (ret)
 		return ret;
 
-	ret = devm_add_action_or_reset(dev, rzg2l_cpg_genpd_remove, genpd);
+	ret = devm_add_action_or_reset(dev, rzg2l_cpg_genpd_remove_simple, &pd->genpd);
 	if (ret)
 		return ret;
 
-	return of_genpd_add_provider_simple(np, genpd);
+	return of_genpd_add_provider_simple(np, &pd->genpd);
+}
+
+static struct generic_pm_domain *
+rzg2l_cpg_pm_domain_xlate(const struct of_phandle_args *spec, void *data)
+{
+	struct generic_pm_domain *domain = ERR_PTR(-ENOENT);
+	struct genpd_onecell_data *genpd = data;
+
+	if (spec->args_count != 1)
+		return ERR_PTR(-EINVAL);
+
+	for (unsigned int i = 0; i < genpd->num_domains; i++) {
+		struct rzg2l_cpg_pd *pd = container_of(genpd->domains[i], struct rzg2l_cpg_pd,
+						       genpd);
+
+		if (pd->id == spec->args[0]) {
+			domain = &pd->genpd;
+			break;
+		}
+	}
+
+	return domain;
+}
+
+static int __init rzg2l_cpg_add_pm_domains(struct rzg2l_cpg_priv *priv)
+{
+	const struct rzg2l_cpg_info *info = priv->info;
+	struct device *dev = priv->dev;
+	struct device_node *np = dev->of_node;
+	struct rzg2l_cpg_pm_domains *domains;
+	struct generic_pm_domain *parent;
+	u32 ncells;
+	int ret;
+
+	ret = of_property_read_u32(np, "#power-domain-cells", &ncells);
+	if (ret)
+		return ret;
+
+	/* For backward compatibility. */
+	if (!ncells)
+		return rzg2l_cpg_add_clk_domain(priv);
+
+	domains = devm_kzalloc(dev, struct_size(domains, domains, info->num_pm_domains),
+			       GFP_KERNEL);
+	if (!domains)
+		return -ENOMEM;
+
+	domains->onecell_data.domains = domains->domains;
+	domains->onecell_data.num_domains = info->num_pm_domains;
+	domains->onecell_data.xlate = rzg2l_cpg_pm_domain_xlate;
+
+	ret = devm_add_action_or_reset(dev, rzg2l_cpg_genpd_remove, &domains->onecell_data);
+	if (ret)
+		return ret;
+
+	for (unsigned int i = 0; i < info->num_pm_domains; i++) {
+		bool always_on = !!(info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON);
+		struct rzg2l_cpg_pd *pd;
+
+		pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
+		if (!pd)
+			return -ENOMEM;
+
+		pd->genpd.name = info->pm_domains[i].name;
+		pd->conf = info->pm_domains[i].conf;
+		pd->id = info->pm_domains[i].id;
+		pd->priv = priv;
+
+		ret = rzg2l_cpg_pd_setup(pd, always_on);
+		if (ret)
+			return ret;
+
+		if (always_on) {
+			ret = rzg2l_cpg_power_on(&pd->genpd);
+			if (ret)
+				return ret;
+		}
+
+		domains->domains[i] = &pd->genpd;
+		/* Parent should be on the very first entry of info->pm_domains[]. */
+		if (!i) {
+			parent = &pd->genpd;
+			continue;
+		}
+
+		ret = pm_genpd_add_subdomain(parent, &pd->genpd);
+		if (ret)
+			return ret;
+	}
+
+	ret = of_genpd_add_provider_onecell(np, &domains->onecell_data);
+	if (ret)
+		return ret;
+
+	/* Prepare for power down the BUSes in power down mode. */
+	if (info->pm_domain_pwrdn_mstop)
+		writel(CPG_PWRDN_MSTOP_ENABLE, priv->base + CPG_PWRDN_MSTOP);
+
+	return 0;
 }
 
 static int __init rzg2l_cpg_probe(struct platform_device *pdev)
@@ -1697,7 +1882,7 @@ static int __init rzg2l_cpg_probe(struct platform_device *pdev)
 	if (error)
 		return error;
 
-	error = rzg2l_cpg_add_clk_domain(priv);
+	error = rzg2l_cpg_add_pm_domains(priv);
 	if (error)
 		return error;
 
diff --git a/drivers/clk/renesas/rzg2l-cpg.h b/drivers/clk/renesas/rzg2l-cpg.h
index 6e38c8fc888c..d9a7357c4873 100644
--- a/drivers/clk/renesas/rzg2l-cpg.h
+++ b/drivers/clk/renesas/rzg2l-cpg.h
@@ -27,6 +27,21 @@
 #define CPG_PL6_ETH_SSEL	(0x418)
 #define CPG_PL5_SDIV		(0x420)
 #define CPG_RST_MON		(0x680)
+#define CPG_BUS_ACPU_MSTOP	(0xB60)
+#define CPG_BUS_MCPU1_MSTOP	(0xB64)
+#define CPG_BUS_MCPU2_MSTOP	(0xB68)
+#define CPG_BUS_PERI_COM_MSTOP	(0xB6C)
+#define CPG_BUS_PERI_CPU_MSTOP	(0xB70)
+#define CPG_BUS_PERI_DDR_MSTOP	(0xB74)
+#define CPG_BUS_REG0_MSTOP	(0xB7C)
+#define CPG_BUS_REG1_MSTOP	(0xB80)
+#define CPG_BUS_TZCDDR_MSTOP	(0xB84)
+#define CPG_MHU_MSTOP		(0xB88)
+#define CPG_BUS_MCPU3_MSTOP	(0xB90)
+#define CPG_BUS_PERI_CPU2_MSTOP	(0xB94)
+#define CPG_PWRDN_IP1		(0xBB0)
+#define CPG_PWRDN_IP2		(0xBB4)
+#define CPG_PWRDN_MSTOP		(0xBC0)
 #define CPG_OTHERFUNC1_REG	(0xBE8)
 
 #define CPG_SIPLL5_STBY_RESETB		BIT(0)
@@ -70,6 +85,8 @@
 
 #define EXTAL_FREQ_IN_MEGA_HZ	(24)
 
+#define CPG_PWRDN_MSTOP_ENABLE		(BIT(16) | BIT(0))
+
 /**
  * Definitions of CPG Core Clocks
  *
@@ -234,6 +251,58 @@ struct rzg2l_reset {
 #define DEF_RST(_id, _off, _bit)	\
 	DEF_RST_MON(_id, _off, _bit, -1)
 
+/**
+ * struct rzg2l_cpg_reg_conf - RZ/G2L register configuration data structure
+ * @off: register offset
+ * @mask: register mask
+ */
+struct rzg2l_cpg_reg_conf {
+	u16 off;
+	u16 mask;
+};
+
+#define DEF_REG_CONF(_off, _mask) ((struct rzg2l_cpg_reg_conf) { .off = (_off), .mask = (_mask) })
+
+/**
+ * struct rzg2l_cpg_pm_domain_conf - PM domain configuration data structure
+ * @mstop: MSTOP register configuration
+ * @pwrdn: PWRDN register configuration
+ */
+struct rzg2l_cpg_pm_domain_conf {
+	struct rzg2l_cpg_reg_conf mstop;
+	struct rzg2l_cpg_reg_conf pwrdn;
+};
+
+/**
+ * struct rzg2l_cpg_pm_domain_init_data - PM domain init data
+ * @name: PM domain name
+ * @conf: PM domain configuration
+ * @flags: RZG2L PM domain flags (see RZG2L_PD_F_*)
+ * @id: PM domain ID (similar to the ones defined in
+ *      include/dt-bindings/clock/<soc-id>-cpg.h)
+ */
+struct rzg2l_cpg_pm_domain_init_data {
+	const char * const name;
+	struct rzg2l_cpg_pm_domain_conf conf;
+	u32 flags;
+	u16 id;
+};
+
+#define DEF_PD(_name, _id, _mstop_conf, _pwrdn_conf, _flags) \
+	{ \
+		.name = (_name), \
+		.id = (_id), \
+		.conf = { \
+			.mstop = (_mstop_conf), \
+			.pwrdn = (_pwrdn_conf), \
+		}, \
+		.flags = (_flags), \
+	}
+
+/* Power domain flags. */
+#define RZG2L_PD_F_ALWAYS_ON	BIT(0)
+#define RZG2L_PD_F_NONE		(0)
+
 /**
  * struct rzg2l_cpg_info - SoC-specific CPG Description
  *
@@ -252,6 +321,9 @@ struct rzg2l_reset {
  * @crit_mod_clks: Array with Module Clock IDs of critical clocks that
  *                 should not be disabled without a knowledgeable driver
  * @num_crit_mod_clks: Number of entries in crit_mod_clks[]
+ * @pm_domains: PM domains init data array
+ * @num_pm_domains: Number of PM domains
+ * @pm_domain_pwrdn_mstop: Specifies if PWRDN MSTOP is supported
  * @has_clk_mon_regs: Flag indicating whether the SoC has CLK_MON registers
  */
 struct rzg2l_cpg_info {
@@ -278,6 +350,11 @@ struct rzg2l_cpg_info {
 	const unsigned int *crit_mod_clks;
 	unsigned int num_crit_mod_clks;
 
+	/* Power domain. */
+	const struct rzg2l_cpg_pm_domain_init_data *pm_domains;
+	unsigned int num_pm_domains;
+	bool pm_domain_pwrdn_mstop;
+
 	bool has_clk_mon_regs;
 };
 
-- 
2.39.2

[PATCH v2 07/10] clk: renesas: r9a08g045: Add support for power domains

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Instantiate power domains for the currently enabled IPs of R9A08G045 SoC.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Change in v2:
- used DEF_REG_CONF() to describe register offests and bits
- updated MSTOP bitmask for ddr domain
- updated MSTOP config for oftde_ddr
- kept the same description for gic as the CPG_BUS_ACPU_MSTOP register
  documentation in the latest HW manual version is wrong and it will be
  fixed; proper description for GIC is located in "Registers for Module
  Standby Mode" table
- haven't added watchdog domain (was missing in v1, too, by mistake) as
  the watchdog restart handler will fail w/o patch [1]; with this pm domain
  support the watchdog will fail to probe; not sure what is the best
  option until [1] will be integrated

[1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20240228083253.2640997-10-claudiu.beznea.uj@bp.renesas.com

 drivers/clk/renesas/r9a08g045-cpg.c | 61 +++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/drivers/clk/renesas/r9a08g045-cpg.c b/drivers/clk/renesas/r9a08g045-cpg.c
index c3e6da2de197..c64769082f5b 100644
--- a/drivers/clk/renesas/r9a08g045-cpg.c
+++ b/drivers/clk/renesas/r9a08g045-cpg.c
@@ -240,6 +240,62 @@ static const unsigned int r9a08g045_crit_mod_clks[] __initconst = {
 	MOD_CLK_BASE + R9A08G045_DMAC_ACLK,
 };
 
+static const struct rzg2l_cpg_pm_domain_init_data r9a08g045_pm_domains[] = {
+	/* Keep always-on domain on the first position for proper domains registration. */
+	DEF_PD("always-on",	R9A08G045_PD_ALWAYS_ON,
+				DEF_REG_CONF(0, 0),
+				DEF_REG_CONF(0, 0),
+				RZG2L_PD_F_ALWAYS_ON),
+	DEF_PD("gic",		R9A08G045_PD_GIC,
+				DEF_REG_CONF(CPG_BUS_ACPU_MSTOP, BIT(3)),
+				DEF_REG_CONF(CPG_PWRDN_IP1, BIT(2)),
+				RZG2L_PD_F_ALWAYS_ON),
+	DEF_PD("ia55",		R9A08G045_PD_IA55,
+				DEF_REG_CONF(CPG_BUS_PERI_CPU_MSTOP, BIT(13)),
+				DEF_REG_CONF(CPG_PWRDN_IP1, BIT(3)),
+				RZG2L_PD_F_ALWAYS_ON),
+	DEF_PD("dmac",		R9A08G045_PD_DMAC,
+				DEF_REG_CONF(CPG_BUS_REG1_MSTOP, GENMASK(3, 0)),
+				DEF_REG_CONF(0, 0),
+				RZG2L_PD_F_ALWAYS_ON),
+	DEF_PD("ddr",		R9A08G045_PD_DDR,
+				DEF_REG_CONF(CPG_BUS_PERI_DDR_MSTOP, GENMASK(1, 0)),
+				DEF_REG_CONF(CPG_PWRDN_IP2, BIT(0)),
+				RZG2L_PD_F_ALWAYS_ON),
+	DEF_PD("tzcddr",	R9A08G045_PD_TZCDDR,
+				DEF_REG_CONF(CPG_BUS_TZCDDR_MSTOP, GENMASK(2, 0)),
+				DEF_REG_CONF(CPG_PWRDN_IP2, BIT(1)),
+				RZG2L_PD_F_ALWAYS_ON),
+	DEF_PD("otfde_ddr",	R9A08G045_PD_OTFDE_DDR,
+				DEF_REG_CONF(CPG_BUS_PERI_CPU2_MSTOP, BIT(2)),
+				DEF_REG_CONF(CPG_PWRDN_IP2, BIT(2)),
+				RZG2L_PD_F_ALWAYS_ON),
+	DEF_PD("sdhi0",		R9A08G045_PD_SDHI0,
+				DEF_REG_CONF(CPG_BUS_PERI_COM_MSTOP, BIT(0)),
+				DEF_REG_CONF(CPG_PWRDN_IP1, BIT(13)),
+				RZG2L_PD_F_NONE),
+	DEF_PD("sdhi1",		R9A08G045_PD_SDHI1,
+				DEF_REG_CONF(CPG_BUS_PERI_COM_MSTOP, BIT(1)),
+				DEF_REG_CONF(CPG_PWRDN_IP1, BIT(14)),
+				RZG2L_PD_F_NONE),
+	DEF_PD("sdhi2",		R9A08G045_PD_SDHI2,
+				DEF_REG_CONF(CPG_BUS_PERI_COM_MSTOP, BIT(11)),
+				DEF_REG_CONF(CPG_PWRDN_IP1, BIT(15)),
+				RZG2L_PD_F_NONE),
+	DEF_PD("eth0",		R9A08G045_PD_ETHER0,
+				DEF_REG_CONF(CPG_BUS_PERI_COM_MSTOP, BIT(2)),
+				DEF_REG_CONF(CPG_PWRDN_IP1, BIT(11)),
+				RZG2L_PD_F_NONE),
+	DEF_PD("eth1",		R9A08G045_PD_ETHER1,
+				DEF_REG_CONF(CPG_BUS_PERI_COM_MSTOP, BIT(3)),
+				DEF_REG_CONF(CPG_PWRDN_IP1, BIT(12)),
+				RZG2L_PD_F_NONE),
+	DEF_PD("scif0",		R9A08G045_PD_SCIF0,
+				DEF_REG_CONF(CPG_BUS_MCPU2_MSTOP, BIT(1)),
+				DEF_REG_CONF(0, 0),
+				RZG2L_PD_F_NONE),
+};
+
 const struct rzg2l_cpg_info r9a08g045_cpg_info = {
 	/* Core Clocks */
 	.core_clks = r9a08g045_core_clks,
@@ -260,5 +316,10 @@ const struct rzg2l_cpg_info r9a08g045_cpg_info = {
 	.resets = r9a08g045_resets,
 	.num_resets = R9A08G045_VBAT_BRESETN + 1, /* Last reset ID + 1 */
 
+	/* Power domains */
+	.pm_domains = r9a08g045_pm_domains,
+	.num_pm_domains = ARRAY_SIZE(r9a08g045_pm_domains),
+	.pm_domain_pwrdn_mstop = true,
+
 	.has_clk_mon_regs = true,
 };
-- 
2.39.2

[PATCH v2 08/10] clk: renesas: rzg2l-cpg: Add suspend/resume support for power domains

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

RZ/G3S supports deep sleep states that it can reach with the help of the
TF-A.

RZ/G3S has a few power domains (e.g. GIC) that need to be always-on while
Linux is running. These domains are initialized (and powered on) when
clock driver is probed.

As the TF-A takes control at the very last(suspend)/first(resume)
phase of configuring the deep sleep state, it can do it's own settings on
power domains.

Thus, to restore the proper Linux state, add rzg2l_cpg_resume() which
powers on the always-on domains and rzg2l_cpg_complete() which activates
the power down mode for the IPs selected through CPG_PWRDN_IP{1, 2}.

Along with it, added the suspend_check member to the RZ/G2L power domain
data structure whose purpose is to checks if a domain can be powered off
while the system is going to suspend. This is necessary for the serial
console domain which needs to be powered on if no_console_suspend is
available in bootargs.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v2:
- none; this patch is new

 drivers/clk/renesas/rzg2l-cpg.c | 66 ++++++++++++++++++++++++++++++---
 drivers/clk/renesas/rzg2l-cpg.h |  1 +
 2 files changed, 62 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c
index b36700f4a9f5..b18af227177e 100644
--- a/drivers/clk/renesas/rzg2l-cpg.c
+++ b/drivers/clk/renesas/rzg2l-cpg.c
@@ -15,6 +15,7 @@
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/clk/renesas.h>
+#include <linux/console.h>
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/init.h>
@@ -139,6 +140,7 @@ struct rzg2l_pll5_mux_dsi_div_param {
  * @num_resets: Number of Module Resets in info->resets[]
  * @last_dt_core_clk: ID of the last Core Clock exported to DT
  * @info: Pointer to platform data
+ * @domains: generic PM domains
  * @mux_dsi_div_params: pll5 mux and dsi div parameters
  */
 struct rzg2l_cpg_priv {
@@ -155,6 +157,8 @@ struct rzg2l_cpg_priv {
 
 	const struct rzg2l_cpg_info *info;
 
+	struct generic_pm_domain **domains;
+
 	struct rzg2l_pll5_mux_dsi_div_param mux_dsi_div_params;
 };
 
@@ -1570,12 +1574,14 @@ struct rzg2l_cpg_pm_domains {
  * struct rzg2l_cpg_pd - RZ/G2L power domain data structure
  * @genpd: generic PM domain
  * @priv: pointer to CPG private data structure
+ * @suspend_check: check if domain could be powered off in suspend
  * @conf: CPG PM domain configuration info
  * @id: RZ/G2L power domain ID
  */
 struct rzg2l_cpg_pd {
 	struct generic_pm_domain genpd;
 	struct rzg2l_cpg_priv *priv;
+	int (*suspend_check)(void);
 	struct rzg2l_cpg_pm_domain_conf conf;
 	u16 id;
 };
@@ -1676,6 +1682,13 @@ static int rzg2l_cpg_power_off(struct generic_pm_domain *domain)
 	struct rzg2l_cpg_reg_conf pwrdn = pd->conf.pwrdn;
 	struct rzg2l_cpg_priv *priv = pd->priv;
 
+	if (pd->suspend_check) {
+		int ret = pd->suspend_check();
+
+		if (ret)
+			return ret;
+	}
+
 	/* Set MSTOP. */
 	if (mstop.mask)
 		writel(mstop.mask | (mstop.mask << 16), priv->base + mstop.off);
@@ -1687,8 +1700,14 @@ static int rzg2l_cpg_power_off(struct generic_pm_domain *domain)
 	return 0;
 }
 
-static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, bool always_on)
+static int rzg2l_pd_suspend_check_console(void)
 {
+	return console_suspend_enabled ? 0 : -EBUSY;
+}
+
+static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, u32 flags)
+{
+	bool always_on = !!(flags & RZG2L_PD_F_ALWAYS_ON);
 	struct dev_power_governor *governor;
 
 	pd->genpd.flags |= GENPD_FLAG_PM_CLK | GENPD_FLAG_ACTIVE_WAKEUP;
@@ -1700,6 +1719,8 @@ static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, bool always_on)
 	} else {
 		pd->genpd.power_on = rzg2l_cpg_power_on;
 		pd->genpd.power_off = rzg2l_cpg_power_off;
+		if (flags & RZG2L_PD_F_CONSOLE)
+			pd->suspend_check = rzg2l_pd_suspend_check_console;
 		governor = &simple_qos_governor;
 	}
 
@@ -1719,7 +1740,7 @@ static int __init rzg2l_cpg_add_clk_domain(struct rzg2l_cpg_priv *priv)
 
 	pd->genpd.name = np->name;
 	pd->priv = priv;
-	ret = rzg2l_cpg_pd_setup(pd, true);
+	ret = rzg2l_cpg_pd_setup(pd, RZG2L_PD_F_ALWAYS_ON);
 	if (ret)
 		return ret;
 
@@ -1778,13 +1799,13 @@ static int __init rzg2l_cpg_add_pm_domains(struct rzg2l_cpg_priv *priv)
 	domains->onecell_data.domains = domains->domains;
 	domains->onecell_data.num_domains = info->num_pm_domains;
 	domains->onecell_data.xlate = rzg2l_cpg_pm_domain_xlate;
+	priv->domains = domains->domains;
 
 	ret = devm_add_action_or_reset(dev, rzg2l_cpg_genpd_remove, &domains->onecell_data);
 	if (ret)
 		return ret;
 
 	for (unsigned int i = 0; i < info->num_pm_domains; i++) {
-		bool always_on = !!(info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON);
 		struct rzg2l_cpg_pd *pd;
 
 		pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
@@ -1796,11 +1817,11 @@ static int __init rzg2l_cpg_add_pm_domains(struct rzg2l_cpg_priv *priv)
 		pd->id = info->pm_domains[i].id;
 		pd->priv = priv;
 
-		ret = rzg2l_cpg_pd_setup(pd, always_on);
+		ret = rzg2l_cpg_pd_setup(pd, info->pm_domains[i].flags);
 		if (ret)
 			return ret;
 
-		if (always_on) {
+		if (info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON) {
 			ret = rzg2l_cpg_power_on(&pd->genpd);
 			if (ret)
 				return ret;
@@ -1890,9 +1911,43 @@ static int __init rzg2l_cpg_probe(struct platform_device *pdev)
 	if (error)
 		return error;
 
+	dev_set_drvdata(dev, priv);
+
 	return 0;
 }
 
+static int rzg2l_cpg_resume(struct device *dev)
+{
+	struct rzg2l_cpg_priv *priv = dev_get_drvdata(dev);
+	const struct rzg2l_cpg_info *info = priv->info;
+
+	/* Power on always ON domains. */
+	for (unsigned int i = 0; i < info->num_pm_domains; i++) {
+		if (info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON) {
+			int ret = rzg2l_cpg_power_on(priv->domains[i]);
+
+			if (ret)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+
+static void rzg2l_cpg_complete(struct device *dev)
+{
+	struct rzg2l_cpg_priv *priv = dev_get_drvdata(dev);
+
+	/* Prepare for power down the BUSes in power down mode. */
+	if (priv->info->pm_domain_pwrdn_mstop)
+		writel(CPG_PWRDN_MSTOP_ENABLE, priv->base + CPG_PWRDN_MSTOP);
+}
+
+static const struct dev_pm_ops rzg2l_cpg_pm_ops = {
+	NOIRQ_SYSTEM_SLEEP_PM_OPS(NULL, rzg2l_cpg_resume)
+	.complete = rzg2l_cpg_complete,
+};
+
 static const struct of_device_id rzg2l_cpg_match[] = {
 #ifdef CONFIG_CLK_R9A07G043
 	{
@@ -1931,6 +1986,7 @@ static struct platform_driver rzg2l_cpg_driver = {
 	.driver		= {
 		.name	= "rzg2l-cpg",
 		.of_match_table = rzg2l_cpg_match,
+		.pm	= pm_sleep_ptr(&rzg2l_cpg_pm_ops),
 	},
 };
 
diff --git a/drivers/clk/renesas/rzg2l-cpg.h b/drivers/clk/renesas/rzg2l-cpg.h
index d9a7357c4873..abff85644270 100644
--- a/drivers/clk/renesas/rzg2l-cpg.h
+++ b/drivers/clk/renesas/rzg2l-cpg.h
@@ -301,6 +301,7 @@ struct rzg2l_cpg_pm_domain_init_data {
 
 /* Power domain flags. */
 #define RZG2L_PD_F_ALWAYS_ON	BIT(0)
+#define RZG2L_PD_F_CONSOLE	BIT(1)
 #define RZG2L_PD_F_NONE		(0)
 
 /**
-- 
2.39.2

[PATCH v2 09/10] clk: renesas: r9a08g045: Add the RZG2L_PD_F_CONSOLE flag to scif0 PM domain

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

The RZG2L_PD_F_CONSOLE is used by the rzg2l-cpg driver to check if the
SCIF domain need to be powered off in suspend. This is necessary when
no_console_suspend is available in bootargs as we want to still see
output on console, thus scif0 domain should remain powered on.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v2:
- none; this patch is new

 drivers/clk/renesas/r9a08g045-cpg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/renesas/r9a08g045-cpg.c b/drivers/clk/renesas/r9a08g045-cpg.c
index c64769082f5b..86d80f357645 100644
--- a/drivers/clk/renesas/r9a08g045-cpg.c
+++ b/drivers/clk/renesas/r9a08g045-cpg.c
@@ -293,7 +293,7 @@ static const struct rzg2l_cpg_pm_domain_init_data r9a08g045_pm_domains[] = {
 	DEF_PD("scif0",		R9A08G045_PD_SCIF0,
 				DEF_REG_CONF(CPG_BUS_MCPU2_MSTOP, BIT(1)),
 				DEF_REG_CONF(0, 0),
-				RZG2L_PD_F_NONE),
+				RZG2L_PD_F_CONSOLE),
 };
 
 const struct rzg2l_cpg_info r9a08g045_cpg_info = {
-- 
2.39.2

[PATCH v2 10/10] arm64: dts: renesas: r9a08g045: Update #power-domain-cells = <1>

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Update CPG #power-domain-cells = <1> and move all the IPs to be part of the
IP specific power domain as the driver has been modified to support
multiple power domains.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v2:
- used proper domain ID for IA55 interrupt controller

 arch/arm64/boot/dts/renesas/r9a08g045.dtsi | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
index 19bbcae01d80..f7a45fca5406 100644
--- a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
@@ -67,7 +67,7 @@ scif0: serial@1004b800 {
 					  "bri", "dri", "tei";
 			clocks = <&cpg CPG_MOD R9A08G045_SCIF0_CLK_PCK>;
 			clock-names = "fck";
-			power-domains = <&cpg>;
+			power-domains = <&cpg R9A08G045_PD_SCIF0>;
 			resets = <&cpg R9A08G045_SCIF0_RST_SYSTEM_N>;
 			status = "disabled";
 		};
@@ -79,7 +79,7 @@ cpg: clock-controller@11010000 {
 			clock-names = "extal";
 			#clock-cells = <2>;
 			#reset-cells = <1>;
-			#power-domain-cells = <0>;
+			#power-domain-cells = <1>;
 		};
 
 		sysc: system-controller@11020000 {
@@ -104,7 +104,7 @@ pinctrl: pinctrl@11030000 {
 			interrupt-parent = <&irqc>;
 			gpio-ranges = <&pinctrl 0 0 152>;
 			clocks = <&cpg CPG_MOD R9A08G045_GPIO_HCLK>;
-			power-domains = <&cpg>;
+			power-domains = <&cpg R9A08G045_PD_ALWAYS_ON>;
 			resets = <&cpg R9A08G045_GPIO_RSTN>,
 				 <&cpg R9A08G045_GPIO_PORT_RESETN>,
 				 <&cpg R9A08G045_GPIO_SPARE_RESETN>;
@@ -173,7 +173,7 @@ irqc: interrupt-controller@11050000 {
 			clocks = <&cpg CPG_MOD R9A08G045_IA55_CLK>,
 				 <&cpg CPG_MOD R9A08G045_IA55_PCLK>;
 			clock-names = "clk", "pclk";
-			power-domains = <&cpg>;
+			power-domains = <&cpg R9A08G045_PD_IA55>;
 			resets = <&cpg R9A08G045_IA55_RESETN>;
 		};
 
@@ -188,7 +188,7 @@ sdhi0: mmc@11c00000  {
 				 <&cpg CPG_MOD R9A08G045_SDHI0_ACLK>;
 			clock-names = "core", "clkh", "cd", "aclk";
 			resets = <&cpg R9A08G045_SDHI0_IXRST>;
-			power-domains = <&cpg>;
+			power-domains = <&cpg R9A08G045_PD_SDHI0>;
 			status = "disabled";
 		};
 
@@ -203,7 +203,7 @@ sdhi1: mmc@11c10000 {
 				 <&cpg CPG_MOD R9A08G045_SDHI1_ACLK>;
 			clock-names = "core", "clkh", "cd", "aclk";
 			resets = <&cpg R9A08G045_SDHI1_IXRST>;
-			power-domains = <&cpg>;
+			power-domains = <&cpg R9A08G045_PD_SDHI1>;
 			status = "disabled";
 		};
 
@@ -218,7 +218,7 @@ sdhi2: mmc@11c20000 {
 				 <&cpg CPG_MOD R9A08G045_SDHI2_ACLK>;
 			clock-names = "core", "clkh", "cd", "aclk";
 			resets = <&cpg R9A08G045_SDHI2_IXRST>;
-			power-domains = <&cpg>;
+			power-domains = <&cpg R9A08G045_PD_SDHI2>;
 			status = "disabled";
 		};
 
@@ -235,7 +235,7 @@ eth0: ethernet@11c30000 {
 				 <&cpg CPG_MOD R9A08G045_ETH0_REFCLK>;
 			clock-names = "axi", "chi", "refclk";
 			resets = <&cpg R9A08G045_ETH0_RST_HW_N>;
-			power-domains = <&cpg>;
+			power-domains = <&cpg R9A08G045_PD_ETHER0>;
 			#address-cells = <1>;
 			#size-cells = <0>;
 			status = "disabled";
@@ -254,7 +254,7 @@ eth1: ethernet@11c40000 {
 				 <&cpg CPG_MOD R9A08G045_ETH1_REFCLK>;
 			clock-names = "axi", "chi", "refclk";
 			resets = <&cpg R9A08G045_ETH1_RST_HW_N>;
-			power-domains = <&cpg>;
+			power-domains = <&cpg R9A08G045_PD_ETHER1>;
 			#address-cells = <1>;
 			#size-cells = <0>;
 			status = "disabled";
@@ -280,7 +280,7 @@ wdt0: watchdog@12800800 {
 				     <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "wdt", "perrout";
 			resets = <&cpg R9A08G045_WDT0_PRESETN>;
-			power-domains = <&cpg>;
+			power-domains = <&cpg R9A08G045_PD_WDT0>;
 			status = "disabled";
 		};
 	};
-- 
2.39.2

Re: [PATCH v2 05/10] dt-bindings: clock: renesas,rzg2l-cpg: Update #power-domain-cells = <1> for RZ/G3S

[Rob Herring]

On Thu, 07 Mar 2024 16:07:23 +0200, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> The driver will be modified (in the next commits) to be able to specify
> individual power domain ID for each IP. The driver will still
> support #power-domain-cells = <0>, thus, previous users are not
> affected.
> 
> The #power-domain-cells = <1> has been instantiated only for RZ/G3S at
> the moment as individual platform clock drivers need to be adapted for
> this to be supported on the rest of the SoCs.
> 
> Also, the description for #power-domain-cells was updated with the links
> to per-SoC power domain IDs.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
> 
> Changes in v2:
> - updated patch title and description
> - kept both 0 and 1 for #power-domain-cells as not all the drivers,
>   device trees are adpated with this series
> - added a reference to dt-bindings/clock/r9a0*-cpg.h for power domain
>   specifiers
> - dropped the changes from examples section
> 
>  .../bindings/clock/renesas,rzg2l-cpg.yaml      | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v2 01/10] dt-bindings: clock: r9a07g043-cpg: Add power domain IDs

[Rob Herring]

On Thu, 07 Mar 2024 16:07:19 +0200, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Add power domain IDs for RZ/G2UL (R9A07G043) SoC.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
> 
> Changes in v2:
> - added "RZ/G2UL Only" comments to some defines
> - added RZ/Five specific defines
> 
>  include/dt-bindings/clock/r9a07g043-cpg.h | 52 +++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
> 

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

Re: [PATCH v2 02/10] dt-bindings: clock: r9a07g044-cpg: Add power domain IDs

[Rob Herring]

On Thu, 07 Mar 2024 16:07:20 +0200, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Add power domain IDs for RZ/G2L (R9A07G044) SoC.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> 
> Changes in v2:
> - collected tag
> 
>  include/dt-bindings/clock/r9a07g044-cpg.h | 58 +++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
> 

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

Re: [PATCH v2 03/10] dt-bindings: clock: r9a07g054-cpg: Add power domain IDs

[Rob Herring]

On Thu, 07 Mar 2024 16:07:21 +0200, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Add power domain IDs for RZ/V2L (R9A07G054) SoC.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> 
> Changes in v2:
> - collected tag
> 
>  include/dt-bindings/clock/r9a07g054-cpg.h | 58 +++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
> 

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

Re: [PATCH v2 04/10] dt-bindings: clock: r9a08g045-cpg: Add power domain IDs

[Rob Herring]

On Thu, 07 Mar 2024 16:07:22 +0200, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Add power domain IDs for RZ/G3S (R9A08G045) SoC.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> 
> Changes in v2:
> - collected tag
> 
>  include/dt-bindings/clock/r9a08g045-cpg.h | 70 +++++++++++++++++++++++
>  1 file changed, 70 insertions(+)
> 

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

Re: [PATCH v2 01/10] dt-bindings: clock: r9a07g043-cpg: Add power domain IDs

[Geert Uytterhoeven]

On Thu, Mar 7, 2024 at 3:07 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Add power domain IDs for RZ/G2UL (R9A07G043) SoC.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v2:
> - added "RZ/G2UL Only" comments to some defines
> - added RZ/Five specific defines

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 05/10] dt-bindings: clock: renesas,rzg2l-cpg: Update #power-domain-cells = <1> for RZ/G3S

[Geert Uytterhoeven]

On Thu, Mar 7, 2024 at 3:07 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> The driver will be modified (in the next commits) to be able to specify
> individual power domain ID for each IP. The driver will still
> support #power-domain-cells = <0>, thus, previous users are not
> affected.
>
> The #power-domain-cells = <1> has been instantiated only for RZ/G3S at
> the moment as individual platform clock drivers need to be adapted for
> this to be supported on the rest of the SoCs.
>
> Also, the description for #power-domain-cells was updated with the links
> to per-SoC power domain IDs.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v2:
> - updated patch title and description
> - kept both 0 and 1 for #power-domain-cells as not all the drivers,
>   device trees are adpated with this series
> - added a reference to dt-bindings/clock/r9a0*-cpg.h for power domain
>   specifiers
> - dropped the changes from examples section

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 06/10] clk: renesas: rzg2l: Extend power domain support

[Geert Uytterhoeven]

Hi Claudiu,

On Thu, Mar 7, 2024 at 3:07 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> RZ/{G2L, V2L, G3S}-based CPG versions have support for saving extra
> power when clocks are disabled by activating module standby. This is done
> through MSTOP-specific registers that are part of CPG. Each individual
> module has one or more bits associated with one MSTOP register (see table
> "Registers for Module Standby Mode" from HW manuals). Hardware manual
> associates modules' clocks with one or more MSTOP bits. There are 3
> mappings available (identified by researching RZ/G2L, RZ/G3S, RZ/V2L HW
> manuals):
>
> case 1: N clocks mapped to N MSTOP bits (with N={0, ..., X})
> case 2: N clocks mapped to 1 MSTOP bit  (with N={0, ..., X})
> case 3: N clocks mapped to M MSTOP bits (with N={0, ..., X}, M={0, ..., Y})
>
> Case 3 has been currently identified on RZ/V2L for the VCPL4 module.
>
> To cover all three cases, the individual platform drivers will provide to
> clock driver MSTOP register offset and associated bits in this register
> as a bitmask and the clock driver will apply this bitmask to proper
> MSTOP register.
>
> Apart from MSTOP support, RZ/G3S can save more power by powering down the
> individual IPs (after MSTOP has been set) if proper bits in
> CPG_PWRDN_IP{1,2} registers are set.
>
> The MSTOP and IP power down support were implemented through power
> domains. Platform-specific clock drivers will register an array of
> type struct rzg2l_cpg_pm_domain_init_data, which will be used to
> instantiate properly the power domains.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v2:
> - called pm_genpd_init() with proper value for is_off argument
> - fixed typos
> - used flexible array for struct rzg2l_cpg_pm_domains::domains member
> - moved genpd member of struct rzg2l_cpg_pd at the beginning of struct
> - didn't initialize the parent variable in rzg2l_cpg_add_pm_domains()
>   as it is already initialized in the for block from
>   rzg2l_cpg_add_pm_domains() and that initialization should be enough
> - dropped RZG2L_PD_F_PARENT flag
> - used datasheet naming for all MSTOP registers
> - added all MSTOP registers to rzg2l-cpg.h
> - reworked the code that initializes the register offset and bits for domains
> - dropped MSTOP*(), PWRDN*() macros and introduced struct rzg2l_cpg_reg_conf
>   and DEF_REG_CONF() for domain description
> - constified the 1st argument of rzg2l_cpg_pm_domain_xlate()
> - used dev instead of priv->dev where possible
> - dropped RZG2L_PD_F_PARENT
> - added RZG2L_PD_F_NONE for better description of domains in platform
>   specific clock drivers

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 07/10] clk: renesas: r9a08g045: Add support for power domains

[Geert Uytterhoeven]

On Thu, Mar 7, 2024 at 3:07 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Instantiate power domains for the currently enabled IPs of R9A08G045 SoC.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Change in v2:
> - used DEF_REG_CONF() to describe register offests and bits
> - updated MSTOP bitmask for ddr domain
> - updated MSTOP config for oftde_ddr
> - kept the same description for gic as the CPG_BUS_ACPU_MSTOP register
>   documentation in the latest HW manual version is wrong and it will be
>   fixed; proper description for GIC is located in "Registers for Module
>   Standby Mode" table
> - haven't added watchdog domain (was missing in v1, too, by mistake) as
>   the watchdog restart handler will fail w/o patch [1]; with this pm domain
>   support the watchdog will fail to probe; not sure what is the best
>   option until [1] will be integrated
>
> [1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20240228083253.2640997-10-claudiu.beznea.uj@bp.renesas.com

I guess we'll have to wait until that dependency is integrated,
or use an immutable branch?

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 10/10] arm64: dts: renesas: r9a08g045: Update #power-domain-cells = <1>

[Geert Uytterhoeven]

On Thu, Mar 7, 2024 at 3:08 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Update CPG #power-domain-cells = <1> and move all the IPs to be part of the
> IP specific power domain as the driver has been modified to support
> multiple power domains.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v2:
> - used proper domain ID for IA55 interrupt controller

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 09/10] clk: renesas: r9a08g045: Add the RZG2L_PD_F_CONSOLE flag to scif0 PM domain

[Geert Uytterhoeven]

Hi Claudiu,

On Thu, Mar 7, 2024 at 3:08 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> The RZG2L_PD_F_CONSOLE is used by the rzg2l-cpg driver to check if the
> SCIF domain need to be powered off in suspend. This is necessary when
> no_console_suspend is available in bootargs as we want to still see
> output on console, thus scif0 domain should remain powered on.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/clk/renesas/r9a08g045-cpg.c
> +++ b/drivers/clk/renesas/r9a08g045-cpg.c
> @@ -293,7 +293,7 @@ static const struct rzg2l_cpg_pm_domain_init_data r9a08g045_pm_domains[] = {
>         DEF_PD("scif0",         R9A08G045_PD_SCIF0,
>                                 DEF_REG_CONF(CPG_BUS_MCPU2_MSTOP, BIT(1)),
>                                 DEF_REG_CONF(0, 0),
> -                               RZG2L_PD_F_NONE),
> +                               RZG2L_PD_F_CONSOLE),

This is not really correct: if SCIF0 is used as the console depends on the
use case, i.e. on chosen/stdout-path.
So this flag should be set at run-time. You can check the power-domains
property of the node pointed to by of_stdout to find the PM Domain that
holds the console, cfr. drivers/pmdomain/renesas/rmobile-sysc.c.

>  };
>
>  const struct rzg2l_cpg_info r9a08g045_cpg_info = {

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 09/10] clk: renesas: r9a08g045: Add the RZG2L_PD_F_CONSOLE flag to scif0 PM domain

[claudiu beznea]

Hi, Geert,

On 14.03.2024 18:06, Geert Uytterhoeven wrote:
> Hi Claudiu,
> 
> On Thu, Mar 7, 2024 at 3:08 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> The RZG2L_PD_F_CONSOLE is used by the rzg2l-cpg driver to check if the
>> SCIF domain need to be powered off in suspend. This is necessary when
>> no_console_suspend is available in bootargs as we want to still see
>> output on console, thus scif0 domain should remain powered on.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Thanks for your patch!
> 
>> --- a/drivers/clk/renesas/r9a08g045-cpg.c
>> +++ b/drivers/clk/renesas/r9a08g045-cpg.c
>> @@ -293,7 +293,7 @@ static const struct rzg2l_cpg_pm_domain_init_data r9a08g045_pm_domains[] = {
>>         DEF_PD("scif0",         R9A08G045_PD_SCIF0,
>>                                 DEF_REG_CONF(CPG_BUS_MCPU2_MSTOP, BIT(1)),
>>                                 DEF_REG_CONF(0, 0),
>> -                               RZG2L_PD_F_NONE),
>> +                               RZG2L_PD_F_CONSOLE),
> 
> This is not really correct: if SCIF0 is used as the console depends on the
> use case, i.e. on chosen/stdout-path.
> So this flag should be set at run-time. You can check the power-domains
> property of the node pointed to by of_stdout to find the PM Domain that
> holds the console, cfr. drivers/pmdomain/renesas/rmobile-sysc.c.

OK, I'll double check.

Thank you,
Claudiu Beznea

> 
>>  };
>>
>>  const struct rzg2l_cpg_info r9a08g045_cpg_info = {
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 

Re: [PATCH v2 08/10] clk: renesas: rzg2l-cpg: Add suspend/resume support for power domains

[Geert Uytterhoeven]

Hi Claudiu,

On Thu, Mar 7, 2024 at 3:07 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> RZ/G3S supports deep sleep states that it can reach with the help of the
> TF-A.
>
> RZ/G3S has a few power domains (e.g. GIC) that need to be always-on while
> Linux is running. These domains are initialized (and powered on) when
> clock driver is probed.
>
> As the TF-A takes control at the very last(suspend)/first(resume)
> phase of configuring the deep sleep state, it can do it's own settings on
> power domains.
>
> Thus, to restore the proper Linux state, add rzg2l_cpg_resume() which
> powers on the always-on domains and rzg2l_cpg_complete() which activates
> the power down mode for the IPs selected through CPG_PWRDN_IP{1, 2}.
>
> Along with it, added the suspend_check member to the RZ/G2L power domain
> data structure whose purpose is to checks if a domain can be powered off
> while the system is going to suspend. This is necessary for the serial
> console domain which needs to be powered on if no_console_suspend is
> available in bootargs.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v2:
> - none; this patch is new

Thanks for your patch!

> --- a/drivers/clk/renesas/rzg2l-cpg.c
> +++ b/drivers/clk/renesas/rzg2l-cpg.c
> @@ -1700,6 +1719,8 @@ static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, bool always_on)
>         } else {
>                 pd->genpd.power_on = rzg2l_cpg_power_on;
>                 pd->genpd.power_off = rzg2l_cpg_power_off;
> +               if (flags & RZG2L_PD_F_CONSOLE)

I think this should be replaced by some dynamic check, cfr. my comments
on PATCH 9/10.

> +                       pd->suspend_check = rzg2l_pd_suspend_check_console;
>                 governor = &simple_qos_governor;
>         }
>

> @@ -1890,9 +1911,43 @@ static int __init rzg2l_cpg_probe(struct platform_device *pdev)
>         if (error)
>                 return error;
>
> +       dev_set_drvdata(dev, priv);
> +
>         return 0;
>  }
>
> +static int rzg2l_cpg_resume(struct device *dev)
> +{
> +       struct rzg2l_cpg_priv *priv = dev_get_drvdata(dev);
> +       const struct rzg2l_cpg_info *info = priv->info;
> +
> +       /* Power on always ON domains. */
> +       for (unsigned int i = 0; i < info->num_pm_domains; i++) {
> +               if (info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON) {

If you would check "priv-domains[i].flags & GENPD_FLAG_ALWAYS_ON"
instead, I think you can make r9a08g045_pm_domains[] __initconst.
You may need to make a copy of the name for pd->genpd.name, though.

> +                       int ret = rzg2l_cpg_power_on(priv->domains[i]);

I assume you are sure none of these domains are enabled by TF/A after
system resume, or by the pmdomain core code?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 08/10] clk: renesas: rzg2l-cpg: Add suspend/resume support for power domains

[claudiu beznea]

Hi, Geert,

Sorry for replying that late to this one.

On 18.03.2024 18:48, Geert Uytterhoeven wrote:
> Hi Claudiu,
> 
> On Thu, Mar 7, 2024 at 3:07 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> RZ/G3S supports deep sleep states that it can reach with the help of the
>> TF-A.
>>
>> RZ/G3S has a few power domains (e.g. GIC) that need to be always-on while
>> Linux is running. These domains are initialized (and powered on) when
>> clock driver is probed.
>>
>> As the TF-A takes control at the very last(suspend)/first(resume)
>> phase of configuring the deep sleep state, it can do it's own settings on
>> power domains.
>>
>> Thus, to restore the proper Linux state, add rzg2l_cpg_resume() which
>> powers on the always-on domains and rzg2l_cpg_complete() which activates
>> the power down mode for the IPs selected through CPG_PWRDN_IP{1, 2}.
>>
>> Along with it, added the suspend_check member to the RZ/G2L power domain
>> data structure whose purpose is to checks if a domain can be powered off
>> while the system is going to suspend. This is necessary for the serial
>> console domain which needs to be powered on if no_console_suspend is
>> available in bootargs.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>> ---
>>
>> Changes in v2:
>> - none; this patch is new
> 
> Thanks for your patch!
> 
>> --- a/drivers/clk/renesas/rzg2l-cpg.c
>> +++ b/drivers/clk/renesas/rzg2l-cpg.c
>> @@ -1700,6 +1719,8 @@ static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, bool always_on)
>>         } else {
>>                 pd->genpd.power_on = rzg2l_cpg_power_on;
>>                 pd->genpd.power_off = rzg2l_cpg_power_off;
>> +               if (flags & RZG2L_PD_F_CONSOLE)
> 
> I think this should be replaced by some dynamic check, cfr. my comments
> on PATCH 9/10.

I agree.

> 
>> +                       pd->suspend_check = rzg2l_pd_suspend_check_console;
>>                 governor = &simple_qos_governor;
>>         }
>>
> 
>> @@ -1890,9 +1911,43 @@ static int __init rzg2l_cpg_probe(struct platform_device *pdev)
>>         if (error)
>>                 return error;
>>
>> +       dev_set_drvdata(dev, priv);
>> +
>>         return 0;
>>  }
>>
>> +static int rzg2l_cpg_resume(struct device *dev)
>> +{
>> +       struct rzg2l_cpg_priv *priv = dev_get_drvdata(dev);
>> +       const struct rzg2l_cpg_info *info = priv->info;
>> +
>> +       /* Power on always ON domains. */
>> +       for (unsigned int i = 0; i < info->num_pm_domains; i++) {
>> +               if (info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON) {
> 
> If you would check "priv-domains[i].flags & GENPD_FLAG_ALWAYS_ON"
> instead, I think you can make r9a08g045_pm_domains[] __initconst.
> You may need to make a copy of the name for pd->genpd.name, though.

I wanted to avoid this copy.

> 
>> +                       int ret = rzg2l_cpg_power_on(priv->domains[i]);
> 
> I assume you are sure none of these domains are enabled by TF/A after
> system resume, or by the pmdomain core code?

Out of TF-A the MSTOP and PWRDN bits for these ones are set and setting
CPG_PWRDN_MSTOP though rzg2l_cpg_complete() leads to system being blocked.
It is the same as in booting case exlained in cover letter.

"the DDR, TZCDDR, OTFDE_DDR were also added, to avoid system being blocked
due to the following lines of code from patch 6/10.

+       /* Prepare for power down the BUSes in power down mode. */
+       if (info->pm_domain_pwrdn_mstop)
+               writel(CPG_PWRDN_MSTOP_ENABLE, priv->base + CPG_PWRDN_MSTOP);

Domain IDs were added to all SoC specific bindings.
"

The PM domain core code doesn't touch these domains while resuming as of my
checkings.

Thank you,
Claudiu Beznea

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 

Re: [PATCH v2 07/10] clk: renesas: r9a08g045: Add support for power domains

[claudiu beznea]

Hi, Geert,

On 14.03.2024 18:01, Geert Uytterhoeven wrote:
> On Thu, Mar 7, 2024 at 3:07 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> Instantiate power domains for the currently enabled IPs of R9A08G045 SoC.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>> ---
>>
>> Change in v2:
>> - used DEF_REG_CONF() to describe register offests and bits
>> - updated MSTOP bitmask for ddr domain
>> - updated MSTOP config for oftde_ddr
>> - kept the same description for gic as the CPG_BUS_ACPU_MSTOP register
>>   documentation in the latest HW manual version is wrong and it will be
>>   fixed; proper description for GIC is located in "Registers for Module
>>   Standby Mode" table
>> - haven't added watchdog domain (was missing in v1, too, by mistake) as
>>   the watchdog restart handler will fail w/o patch [1]; with this pm domain
>>   support the watchdog will fail to probe; not sure what is the best
>>   option until [1] will be integrated
>>
>> [1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20240228083253.2640997-10-claudiu.beznea.uj@bp.renesas.com
> 
> I guess we'll have to wait until that dependency is integrated,

I opt for this option to not break the reset support currently integrated.
I don't have any feedback from maintainers yet on [1], though. I don't know
how long it will take.

Thank you,
Claudiu Beznea


> or use an immutable branch?
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

Re: [PATCH v2 08/10] clk: renesas: rzg2l-cpg: Add suspend/resume support for power domains

[Ulf Hansson]

On Thu, 7 Mar 2024 at 15:10, Claudiu <claudiu.beznea@tuxon.dev> wrote:
>
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> RZ/G3S supports deep sleep states that it can reach with the help of the
> TF-A.
>
> RZ/G3S has a few power domains (e.g. GIC) that need to be always-on while
> Linux is running. These domains are initialized (and powered on) when
> clock driver is probed.
>
> As the TF-A takes control at the very last(suspend)/first(resume)
> phase of configuring the deep sleep state, it can do it's own settings on
> power domains.

For my understanding, can you please elaborate on this part a bit.
What does the "last suspend/resume phase" mean, more exactly, here?

>
> Thus, to restore the proper Linux state, add rzg2l_cpg_resume() which
> powers on the always-on domains and rzg2l_cpg_complete() which activates
> the power down mode for the IPs selected through CPG_PWRDN_IP{1, 2}.
>
> Along with it, added the suspend_check member to the RZ/G2L power domain
> data structure whose purpose is to checks if a domain can be powered off
> while the system is going to suspend. This is necessary for the serial
> console domain which needs to be powered on if no_console_suspend is
> available in bootargs.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v2:
> - none; this patch is new
>
>  drivers/clk/renesas/rzg2l-cpg.c | 66 ++++++++++++++++++++++++++++++---
>  drivers/clk/renesas/rzg2l-cpg.h |  1 +
>  2 files changed, 62 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c
> index b36700f4a9f5..b18af227177e 100644
> --- a/drivers/clk/renesas/rzg2l-cpg.c
> +++ b/drivers/clk/renesas/rzg2l-cpg.c
> @@ -15,6 +15,7 @@
>  #include <linux/clk.h>
>  #include <linux/clk-provider.h>
>  #include <linux/clk/renesas.h>
> +#include <linux/console.h>
>  #include <linux/delay.h>
>  #include <linux/device.h>
>  #include <linux/init.h>
> @@ -139,6 +140,7 @@ struct rzg2l_pll5_mux_dsi_div_param {
>   * @num_resets: Number of Module Resets in info->resets[]
>   * @last_dt_core_clk: ID of the last Core Clock exported to DT
>   * @info: Pointer to platform data
> + * @domains: generic PM domains
>   * @mux_dsi_div_params: pll5 mux and dsi div parameters
>   */
>  struct rzg2l_cpg_priv {
> @@ -155,6 +157,8 @@ struct rzg2l_cpg_priv {
>
>         const struct rzg2l_cpg_info *info;
>
> +       struct generic_pm_domain **domains;
> +
>         struct rzg2l_pll5_mux_dsi_div_param mux_dsi_div_params;
>  };
>
> @@ -1570,12 +1574,14 @@ struct rzg2l_cpg_pm_domains {
>   * struct rzg2l_cpg_pd - RZ/G2L power domain data structure
>   * @genpd: generic PM domain
>   * @priv: pointer to CPG private data structure
> + * @suspend_check: check if domain could be powered off in suspend
>   * @conf: CPG PM domain configuration info
>   * @id: RZ/G2L power domain ID
>   */
>  struct rzg2l_cpg_pd {
>         struct generic_pm_domain genpd;
>         struct rzg2l_cpg_priv *priv;
> +       int (*suspend_check)(void);
>         struct rzg2l_cpg_pm_domain_conf conf;
>         u16 id;
>  };
> @@ -1676,6 +1682,13 @@ static int rzg2l_cpg_power_off(struct generic_pm_domain *domain)
>         struct rzg2l_cpg_reg_conf pwrdn = pd->conf.pwrdn;
>         struct rzg2l_cpg_priv *priv = pd->priv;
>
> +       if (pd->suspend_check) {
> +               int ret = pd->suspend_check();
> +
> +               if (ret)
> +                       return ret;
> +       }
> +

This should not be needed at all, I think.

Instead, genpd should be able to take the correct decision during
system-wide suspend and simply avoid calling the ->power_off()
callback, when that is needed.

If I understand correctly, GENPD_FLAG_ACTIVE_WAKEUP is set for the
genpd in question. The only remaining thing would then be to let the
console driver, during system suspend, check whether
"console_suspend_enabled" is set and then call device_set_awake_path()
for its device. In this way, genpd should then keep the corresponding
PM domain powered-on.

>         /* Set MSTOP. */
>         if (mstop.mask)
>                 writel(mstop.mask | (mstop.mask << 16), priv->base + mstop.off);
> @@ -1687,8 +1700,14 @@ static int rzg2l_cpg_power_off(struct generic_pm_domain *domain)
>         return 0;
>  }
>
> -static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, bool always_on)
> +static int rzg2l_pd_suspend_check_console(void)
>  {
> +       return console_suspend_enabled ? 0 : -EBUSY;
> +}
> +
> +static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, u32 flags)
> +{
> +       bool always_on = !!(flags & RZG2L_PD_F_ALWAYS_ON);
>         struct dev_power_governor *governor;
>
>         pd->genpd.flags |= GENPD_FLAG_PM_CLK | GENPD_FLAG_ACTIVE_WAKEUP;
> @@ -1700,6 +1719,8 @@ static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, bool always_on)
>         } else {
>                 pd->genpd.power_on = rzg2l_cpg_power_on;
>                 pd->genpd.power_off = rzg2l_cpg_power_off;
> +               if (flags & RZG2L_PD_F_CONSOLE)
> +                       pd->suspend_check = rzg2l_pd_suspend_check_console;
>                 governor = &simple_qos_governor;
>         }
>
> @@ -1719,7 +1740,7 @@ static int __init rzg2l_cpg_add_clk_domain(struct rzg2l_cpg_priv *priv)
>
>         pd->genpd.name = np->name;
>         pd->priv = priv;
> -       ret = rzg2l_cpg_pd_setup(pd, true);
> +       ret = rzg2l_cpg_pd_setup(pd, RZG2L_PD_F_ALWAYS_ON);
>         if (ret)
>                 return ret;
>
> @@ -1778,13 +1799,13 @@ static int __init rzg2l_cpg_add_pm_domains(struct rzg2l_cpg_priv *priv)
>         domains->onecell_data.domains = domains->domains;
>         domains->onecell_data.num_domains = info->num_pm_domains;
>         domains->onecell_data.xlate = rzg2l_cpg_pm_domain_xlate;
> +       priv->domains = domains->domains;
>
>         ret = devm_add_action_or_reset(dev, rzg2l_cpg_genpd_remove, &domains->onecell_data);
>         if (ret)
>                 return ret;
>
>         for (unsigned int i = 0; i < info->num_pm_domains; i++) {
> -               bool always_on = !!(info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON);
>                 struct rzg2l_cpg_pd *pd;
>
>                 pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
> @@ -1796,11 +1817,11 @@ static int __init rzg2l_cpg_add_pm_domains(struct rzg2l_cpg_priv *priv)
>                 pd->id = info->pm_domains[i].id;
>                 pd->priv = priv;
>
> -               ret = rzg2l_cpg_pd_setup(pd, always_on);
> +               ret = rzg2l_cpg_pd_setup(pd, info->pm_domains[i].flags);
>                 if (ret)
>                         return ret;
>
> -               if (always_on) {
> +               if (info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON) {
>                         ret = rzg2l_cpg_power_on(&pd->genpd);
>                         if (ret)
>                                 return ret;
> @@ -1890,9 +1911,43 @@ static int __init rzg2l_cpg_probe(struct platform_device *pdev)
>         if (error)
>                 return error;
>
> +       dev_set_drvdata(dev, priv);
> +
>         return 0;
>  }
>
> +static int rzg2l_cpg_resume(struct device *dev)
> +{
> +       struct rzg2l_cpg_priv *priv = dev_get_drvdata(dev);
> +       const struct rzg2l_cpg_info *info = priv->info;
> +
> +       /* Power on always ON domains. */
> +       for (unsigned int i = 0; i < info->num_pm_domains; i++) {
> +               if (info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON) {
> +                       int ret = rzg2l_cpg_power_on(priv->domains[i]);
> +
> +                       if (ret)
> +                               return ret;
> +               }
> +       }

I don't quite understand why this is needed? Is always-on PM domains
being powered-off during system wide suspend, so you need to power
them on again?

> +
> +       return 0;
> +}
> +
> +static void rzg2l_cpg_complete(struct device *dev)
> +{
> +       struct rzg2l_cpg_priv *priv = dev_get_drvdata(dev);
> +
> +       /* Prepare for power down the BUSes in power down mode. */
> +       if (priv->info->pm_domain_pwrdn_mstop)
> +               writel(CPG_PWRDN_MSTOP_ENABLE, priv->base + CPG_PWRDN_MSTOP);
> +}
> +
> +static const struct dev_pm_ops rzg2l_cpg_pm_ops = {
> +       NOIRQ_SYSTEM_SLEEP_PM_OPS(NULL, rzg2l_cpg_resume)
> +       .complete = rzg2l_cpg_complete,
> +};
> +
>  static const struct of_device_id rzg2l_cpg_match[] = {
>  #ifdef CONFIG_CLK_R9A07G043
>         {
> @@ -1931,6 +1986,7 @@ static struct platform_driver rzg2l_cpg_driver = {
>         .driver         = {
>                 .name   = "rzg2l-cpg",
>                 .of_match_table = rzg2l_cpg_match,
> +               .pm     = pm_sleep_ptr(&rzg2l_cpg_pm_ops),
>         },
>  };
>
> diff --git a/drivers/clk/renesas/rzg2l-cpg.h b/drivers/clk/renesas/rzg2l-cpg.h
> index d9a7357c4873..abff85644270 100644
> --- a/drivers/clk/renesas/rzg2l-cpg.h
> +++ b/drivers/clk/renesas/rzg2l-cpg.h
> @@ -301,6 +301,7 @@ struct rzg2l_cpg_pm_domain_init_data {
>
>  /* Power domain flags. */
>  #define RZG2L_PD_F_ALWAYS_ON   BIT(0)
> +#define RZG2L_PD_F_CONSOLE     BIT(1)
>  #define RZG2L_PD_F_NONE                (0)
>

Kind regards
Uffe

Re: [PATCH v2 08/10] clk: renesas: rzg2l-cpg: Add suspend/resume support for power domains

[claudiu beznea]

Hi, Ulf,

On 16.04.2024 15:07, Ulf Hansson wrote:
> On Thu, 7 Mar 2024 at 15:10, Claudiu <claudiu.beznea@tuxon.dev> wrote:
>>
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> RZ/G3S supports deep sleep states that it can reach with the help of the
>> TF-A.
>>
>> RZ/G3S has a few power domains (e.g. GIC) that need to be always-on while
>> Linux is running. These domains are initialized (and powered on) when
>> clock driver is probed.
>>
>> As the TF-A takes control at the very last(suspend)/first(resume)
>> phase of configuring the deep sleep state, it can do it's own settings on
>> power domains.
> 
> For my understanding, can you please elaborate on this part a bit.
> What does the "last suspend/resume phase" mean, more exactly, here?

The RZ/G3S SoC support a power saving mode where most of the SoC parts are
turned off and the system RAM is switched to retention mode. This is done
with the help of TF-A. The handshake b/w Linux and TF-A is done though the
drivers/firmware/psci/psci.c driver.

After Linux finishes the execution of suspend code the control is taken by
TF-A. TF-A does the final settings on the system (e.g. switching the RAM to
retention mode) and power off most of the SoC parts.

By the last phase of the suspend I'm referring to the TF-A doing the final
adjustments for the system to switch to this power saving mode.

When resuming, as the TF-A is the 1st one being executed on the system
(this is what I called above the 1st phase of the resume), TF-A moves the
DDR out of retention mode, reconfigure basic IPs (like in boot case as most
of the SoC parts were powered off) and then give the control to Linux which
will execute the resume code.


> 
>>
>> Thus, to restore the proper Linux state, add rzg2l_cpg_resume() which
>> powers on the always-on domains and rzg2l_cpg_complete() which activates
>> the power down mode for the IPs selected through CPG_PWRDN_IP{1, 2}.
>>
>> Along with it, added the suspend_check member to the RZ/G2L power domain
>> data structure whose purpose is to checks if a domain can be powered off
>> while the system is going to suspend. This is necessary for the serial
>> console domain which needs to be powered on if no_console_suspend is
>> available in bootargs.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>> ---
>>
>> Changes in v2:
>> - none; this patch is new
>>
>>  drivers/clk/renesas/rzg2l-cpg.c | 66 ++++++++++++++++++++++++++++++---
>>  drivers/clk/renesas/rzg2l-cpg.h |  1 +
>>  2 files changed, 62 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c
>> index b36700f4a9f5..b18af227177e 100644
>> --- a/drivers/clk/renesas/rzg2l-cpg.c
>> +++ b/drivers/clk/renesas/rzg2l-cpg.c
>> @@ -15,6 +15,7 @@
>>  #include <linux/clk.h>
>>  #include <linux/clk-provider.h>
>>  #include <linux/clk/renesas.h>
>> +#include <linux/console.h>
>>  #include <linux/delay.h>
>>  #include <linux/device.h>
>>  #include <linux/init.h>
>> @@ -139,6 +140,7 @@ struct rzg2l_pll5_mux_dsi_div_param {
>>   * @num_resets: Number of Module Resets in info->resets[]
>>   * @last_dt_core_clk: ID of the last Core Clock exported to DT
>>   * @info: Pointer to platform data
>> + * @domains: generic PM domains
>>   * @mux_dsi_div_params: pll5 mux and dsi div parameters
>>   */
>>  struct rzg2l_cpg_priv {
>> @@ -155,6 +157,8 @@ struct rzg2l_cpg_priv {
>>
>>         const struct rzg2l_cpg_info *info;
>>
>> +       struct generic_pm_domain **domains;
>> +
>>         struct rzg2l_pll5_mux_dsi_div_param mux_dsi_div_params;
>>  };
>>
>> @@ -1570,12 +1574,14 @@ struct rzg2l_cpg_pm_domains {
>>   * struct rzg2l_cpg_pd - RZ/G2L power domain data structure
>>   * @genpd: generic PM domain
>>   * @priv: pointer to CPG private data structure
>> + * @suspend_check: check if domain could be powered off in suspend
>>   * @conf: CPG PM domain configuration info
>>   * @id: RZ/G2L power domain ID
>>   */
>>  struct rzg2l_cpg_pd {
>>         struct generic_pm_domain genpd;
>>         struct rzg2l_cpg_priv *priv;
>> +       int (*suspend_check)(void);
>>         struct rzg2l_cpg_pm_domain_conf conf;
>>         u16 id;
>>  };
>> @@ -1676,6 +1682,13 @@ static int rzg2l_cpg_power_off(struct generic_pm_domain *domain)
>>         struct rzg2l_cpg_reg_conf pwrdn = pd->conf.pwrdn;
>>         struct rzg2l_cpg_priv *priv = pd->priv;
>>
>> +       if (pd->suspend_check) {
>> +               int ret = pd->suspend_check();
>> +
>> +               if (ret)
>> +                       return ret;
>> +       }
>> +
> 
> This should not be needed at all, I think.
> 
> Instead, genpd should be able to take the correct decision during
> system-wide suspend and simply avoid calling the ->power_off()
> callback, when that is needed.
> 
> If I understand correctly, GENPD_FLAG_ACTIVE_WAKEUP is set for the
> genpd in question. The only remaining thing would then be to let the
> console driver, during system suspend, check whether
> "console_suspend_enabled" is set and then call device_set_awake_path()
> for its device. In this way, genpd should then keep the corresponding
> PM domain powered-on.

You're right! I've checked it and all good w/o ->suspend_check() if
device_set_wakeup_path() is called for the console driver.

I'll send an update for it.

> 
>>         /* Set MSTOP. */
>>         if (mstop.mask)
>>                 writel(mstop.mask | (mstop.mask << 16), priv->base + mstop.off);
>> @@ -1687,8 +1700,14 @@ static int rzg2l_cpg_power_off(struct generic_pm_domain *domain)
>>         return 0;
>>  }
>>
>> -static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, bool always_on)
>> +static int rzg2l_pd_suspend_check_console(void)
>>  {
>> +       return console_suspend_enabled ? 0 : -EBUSY;
>> +}
>> +
>> +static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, u32 flags)
>> +{
>> +       bool always_on = !!(flags & RZG2L_PD_F_ALWAYS_ON);
>>         struct dev_power_governor *governor;
>>
>>         pd->genpd.flags |= GENPD_FLAG_PM_CLK | GENPD_FLAG_ACTIVE_WAKEUP;
>> @@ -1700,6 +1719,8 @@ static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, bool always_on)
>>         } else {
>>                 pd->genpd.power_on = rzg2l_cpg_power_on;
>>                 pd->genpd.power_off = rzg2l_cpg_power_off;
>> +               if (flags & RZG2L_PD_F_CONSOLE)
>> +                       pd->suspend_check = rzg2l_pd_suspend_check_console;
>>                 governor = &simple_qos_governor;
>>         }
>>
>> @@ -1719,7 +1740,7 @@ static int __init rzg2l_cpg_add_clk_domain(struct rzg2l_cpg_priv *priv)
>>
>>         pd->genpd.name = np->name;
>>         pd->priv = priv;
>> -       ret = rzg2l_cpg_pd_setup(pd, true);
>> +       ret = rzg2l_cpg_pd_setup(pd, RZG2L_PD_F_ALWAYS_ON);
>>         if (ret)
>>                 return ret;
>>
>> @@ -1778,13 +1799,13 @@ static int __init rzg2l_cpg_add_pm_domains(struct rzg2l_cpg_priv *priv)
>>         domains->onecell_data.domains = domains->domains;
>>         domains->onecell_data.num_domains = info->num_pm_domains;
>>         domains->onecell_data.xlate = rzg2l_cpg_pm_domain_xlate;
>> +       priv->domains = domains->domains;
>>
>>         ret = devm_add_action_or_reset(dev, rzg2l_cpg_genpd_remove, &domains->onecell_data);
>>         if (ret)
>>                 return ret;
>>
>>         for (unsigned int i = 0; i < info->num_pm_domains; i++) {
>> -               bool always_on = !!(info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON);
>>                 struct rzg2l_cpg_pd *pd;
>>
>>                 pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
>> @@ -1796,11 +1817,11 @@ static int __init rzg2l_cpg_add_pm_domains(struct rzg2l_cpg_priv *priv)
>>                 pd->id = info->pm_domains[i].id;
>>                 pd->priv = priv;
>>
>> -               ret = rzg2l_cpg_pd_setup(pd, always_on);
>> +               ret = rzg2l_cpg_pd_setup(pd, info->pm_domains[i].flags);
>>                 if (ret)
>>                         return ret;
>>
>> -               if (always_on) {
>> +               if (info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON) {
>>                         ret = rzg2l_cpg_power_on(&pd->genpd);
>>                         if (ret)
>>                                 return ret;
>> @@ -1890,9 +1911,43 @@ static int __init rzg2l_cpg_probe(struct platform_device *pdev)
>>         if (error)
>>                 return error;
>>
>> +       dev_set_drvdata(dev, priv);
>> +
>>         return 0;
>>  }
>>
>> +static int rzg2l_cpg_resume(struct device *dev)
>> +{
>> +       struct rzg2l_cpg_priv *priv = dev_get_drvdata(dev);
>> +       const struct rzg2l_cpg_info *info = priv->info;
>> +
>> +       /* Power on always ON domains. */
>> +       for (unsigned int i = 0; i < info->num_pm_domains; i++) {
>> +               if (info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON) {
>> +                       int ret = rzg2l_cpg_power_on(priv->domains[i]);
>> +
>> +                       if (ret)
>> +                               return ret;
>> +               }
>> +       }
> 
> I don't quite understand why this is needed? Is always-on PM domains
> being powered-off during system wide suspend, so you need to power
> them on again?

Yes, as power to most of the system parts is cut off during sytem suspend
(and DDR is kept in retention mode) and the resume is almost like a cold
boot where the TF-A does basic re-initialization and then pass execution to
 Linux resume code.

Thank you,
Claudiu Beznea

> 
>> +
>> +       return 0;
>> +}
>> +
>> +static void rzg2l_cpg_complete(struct device *dev)
>> +{
>> +       struct rzg2l_cpg_priv *priv = dev_get_drvdata(dev);
>> +
>> +       /* Prepare for power down the BUSes in power down mode. */
>> +       if (priv->info->pm_domain_pwrdn_mstop)
>> +               writel(CPG_PWRDN_MSTOP_ENABLE, priv->base + CPG_PWRDN_MSTOP);
>> +}
>> +
>> +static const struct dev_pm_ops rzg2l_cpg_pm_ops = {
>> +       NOIRQ_SYSTEM_SLEEP_PM_OPS(NULL, rzg2l_cpg_resume)
>> +       .complete = rzg2l_cpg_complete,
>> +};
>> +
>>  static const struct of_device_id rzg2l_cpg_match[] = {
>>  #ifdef CONFIG_CLK_R9A07G043
>>         {
>> @@ -1931,6 +1986,7 @@ static struct platform_driver rzg2l_cpg_driver = {
>>         .driver         = {
>>                 .name   = "rzg2l-cpg",
>>                 .of_match_table = rzg2l_cpg_match,
>> +               .pm     = pm_sleep_ptr(&rzg2l_cpg_pm_ops),
>>         },
>>  };
>>
>> diff --git a/drivers/clk/renesas/rzg2l-cpg.h b/drivers/clk/renesas/rzg2l-cpg.h
>> index d9a7357c4873..abff85644270 100644
>> --- a/drivers/clk/renesas/rzg2l-cpg.h
>> +++ b/drivers/clk/renesas/rzg2l-cpg.h
>> @@ -301,6 +301,7 @@ struct rzg2l_cpg_pm_domain_init_data {
>>
>>  /* Power domain flags. */
>>  #define RZG2L_PD_F_ALWAYS_ON   BIT(0)
>> +#define RZG2L_PD_F_CONSOLE     BIT(1)
>>  #define RZG2L_PD_F_NONE                (0)
>>
> 
> Kind regards
> Uffe

Re: [PATCH v2 08/10] clk: renesas: rzg2l-cpg: Add suspend/resume support for power domains

[Ulf Hansson]

On Wed, 17 Apr 2024 at 10:05, claudiu beznea <claudiu.beznea@tuxon.dev> wrote:
>
> Hi, Ulf,
>
> On 16.04.2024 15:07, Ulf Hansson wrote:
> > On Thu, 7 Mar 2024 at 15:10, Claudiu <claudiu.beznea@tuxon.dev> wrote:
> >>
> >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >>
> >> RZ/G3S supports deep sleep states that it can reach with the help of the
> >> TF-A.
> >>
> >> RZ/G3S has a few power domains (e.g. GIC) that need to be always-on while
> >> Linux is running. These domains are initialized (and powered on) when
> >> clock driver is probed.
> >>
> >> As the TF-A takes control at the very last(suspend)/first(resume)
> >> phase of configuring the deep sleep state, it can do it's own settings on
> >> power domains.
> >
> > For my understanding, can you please elaborate on this part a bit.
> > What does the "last suspend/resume phase" mean, more exactly, here?
>
> The RZ/G3S SoC support a power saving mode where most of the SoC parts are
> turned off and the system RAM is switched to retention mode. This is done
> with the help of TF-A. The handshake b/w Linux and TF-A is done though the
> drivers/firmware/psci/psci.c driver.
>
> After Linux finishes the execution of suspend code the control is taken by
> TF-A. TF-A does the final settings on the system (e.g. switching the RAM to
> retention mode) and power off most of the SoC parts.
>
> By the last phase of the suspend I'm referring to the TF-A doing the final
> adjustments for the system to switch to this power saving mode.
>
> When resuming, as the TF-A is the 1st one being executed on the system
> (this is what I called above the 1st phase of the resume), TF-A moves the
> DDR out of retention mode, reconfigure basic IPs (like in boot case as most
> of the SoC parts were powered off) and then give the control to Linux which
> will execute the resume code.

Alright, thanks for clarifying! This makes sense to me now!

>
>
> >
> >>
> >> Thus, to restore the proper Linux state, add rzg2l_cpg_resume() which
> >> powers on the always-on domains and rzg2l_cpg_complete() which activates
> >> the power down mode for the IPs selected through CPG_PWRDN_IP{1, 2}.
> >>
> >> Along with it, added the suspend_check member to the RZ/G2L power domain
> >> data structure whose purpose is to checks if a domain can be powered off
> >> while the system is going to suspend. This is necessary for the serial
> >> console domain which needs to be powered on if no_console_suspend is
> >> available in bootargs.
> >>
> >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >> ---
> >>
> >> Changes in v2:
> >> - none; this patch is new
> >>
> >>  drivers/clk/renesas/rzg2l-cpg.c | 66 ++++++++++++++++++++++++++++++---
> >>  drivers/clk/renesas/rzg2l-cpg.h |  1 +
> >>  2 files changed, 62 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c
> >> index b36700f4a9f5..b18af227177e 100644
> >> --- a/drivers/clk/renesas/rzg2l-cpg.c
> >> +++ b/drivers/clk/renesas/rzg2l-cpg.c
> >> @@ -15,6 +15,7 @@
> >>  #include <linux/clk.h>
> >>  #include <linux/clk-provider.h>
> >>  #include <linux/clk/renesas.h>
> >> +#include <linux/console.h>
> >>  #include <linux/delay.h>
> >>  #include <linux/device.h>
> >>  #include <linux/init.h>
> >> @@ -139,6 +140,7 @@ struct rzg2l_pll5_mux_dsi_div_param {
> >>   * @num_resets: Number of Module Resets in info->resets[]
> >>   * @last_dt_core_clk: ID of the last Core Clock exported to DT
> >>   * @info: Pointer to platform data
> >> + * @domains: generic PM domains
> >>   * @mux_dsi_div_params: pll5 mux and dsi div parameters
> >>   */
> >>  struct rzg2l_cpg_priv {
> >> @@ -155,6 +157,8 @@ struct rzg2l_cpg_priv {
> >>
> >>         const struct rzg2l_cpg_info *info;
> >>
> >> +       struct generic_pm_domain **domains;
> >> +
> >>         struct rzg2l_pll5_mux_dsi_div_param mux_dsi_div_params;
> >>  };
> >>
> >> @@ -1570,12 +1574,14 @@ struct rzg2l_cpg_pm_domains {
> >>   * struct rzg2l_cpg_pd - RZ/G2L power domain data structure
> >>   * @genpd: generic PM domain
> >>   * @priv: pointer to CPG private data structure
> >> + * @suspend_check: check if domain could be powered off in suspend
> >>   * @conf: CPG PM domain configuration info
> >>   * @id: RZ/G2L power domain ID
> >>   */
> >>  struct rzg2l_cpg_pd {
> >>         struct generic_pm_domain genpd;
> >>         struct rzg2l_cpg_priv *priv;
> >> +       int (*suspend_check)(void);
> >>         struct rzg2l_cpg_pm_domain_conf conf;
> >>         u16 id;
> >>  };
> >> @@ -1676,6 +1682,13 @@ static int rzg2l_cpg_power_off(struct generic_pm_domain *domain)
> >>         struct rzg2l_cpg_reg_conf pwrdn = pd->conf.pwrdn;
> >>         struct rzg2l_cpg_priv *priv = pd->priv;
> >>
> >> +       if (pd->suspend_check) {
> >> +               int ret = pd->suspend_check();
> >> +
> >> +               if (ret)
> >> +                       return ret;
> >> +       }
> >> +
> >
> > This should not be needed at all, I think.
> >
> > Instead, genpd should be able to take the correct decision during
> > system-wide suspend and simply avoid calling the ->power_off()
> > callback, when that is needed.
> >
> > If I understand correctly, GENPD_FLAG_ACTIVE_WAKEUP is set for the
> > genpd in question. The only remaining thing would then be to let the
> > console driver, during system suspend, check whether
> > "console_suspend_enabled" is set and then call device_set_awake_path()
> > for its device. In this way, genpd should then keep the corresponding
> > PM domain powered-on.
>
> You're right! I've checked it and all good w/o ->suspend_check() if
> device_set_wakeup_path() is called for the console driver.
>
> I'll send an update for it.

Great! Please keep me posted on the entire series for next version. I
will try to continue to help review this.

>
> >
> >>         /* Set MSTOP. */
> >>         if (mstop.mask)
> >>                 writel(mstop.mask | (mstop.mask << 16), priv->base + mstop.off);
> >> @@ -1687,8 +1700,14 @@ static int rzg2l_cpg_power_off(struct generic_pm_domain *domain)
> >>         return 0;
> >>  }
> >>
> >> -static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, bool always_on)
> >> +static int rzg2l_pd_suspend_check_console(void)
> >>  {
> >> +       return console_suspend_enabled ? 0 : -EBUSY;
> >> +}
> >> +
> >> +static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, u32 flags)
> >> +{
> >> +       bool always_on = !!(flags & RZG2L_PD_F_ALWAYS_ON);
> >>         struct dev_power_governor *governor;
> >>
> >>         pd->genpd.flags |= GENPD_FLAG_PM_CLK | GENPD_FLAG_ACTIVE_WAKEUP;
> >> @@ -1700,6 +1719,8 @@ static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, bool always_on)
> >>         } else {
> >>                 pd->genpd.power_on = rzg2l_cpg_power_on;
> >>                 pd->genpd.power_off = rzg2l_cpg_power_off;
> >> +               if (flags & RZG2L_PD_F_CONSOLE)
> >> +                       pd->suspend_check = rzg2l_pd_suspend_check_console;
> >>                 governor = &simple_qos_governor;
> >>         }
> >>
> >> @@ -1719,7 +1740,7 @@ static int __init rzg2l_cpg_add_clk_domain(struct rzg2l_cpg_priv *priv)
> >>
> >>         pd->genpd.name = np->name;
> >>         pd->priv = priv;
> >> -       ret = rzg2l_cpg_pd_setup(pd, true);
> >> +       ret = rzg2l_cpg_pd_setup(pd, RZG2L_PD_F_ALWAYS_ON);
> >>         if (ret)
> >>                 return ret;
> >>
> >> @@ -1778,13 +1799,13 @@ static int __init rzg2l_cpg_add_pm_domains(struct rzg2l_cpg_priv *priv)
> >>         domains->onecell_data.domains = domains->domains;
> >>         domains->onecell_data.num_domains = info->num_pm_domains;
> >>         domains->onecell_data.xlate = rzg2l_cpg_pm_domain_xlate;
> >> +       priv->domains = domains->domains;
> >>
> >>         ret = devm_add_action_or_reset(dev, rzg2l_cpg_genpd_remove, &domains->onecell_data);
> >>         if (ret)
> >>                 return ret;
> >>
> >>         for (unsigned int i = 0; i < info->num_pm_domains; i++) {
> >> -               bool always_on = !!(info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON);
> >>                 struct rzg2l_cpg_pd *pd;
> >>
> >>                 pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
> >> @@ -1796,11 +1817,11 @@ static int __init rzg2l_cpg_add_pm_domains(struct rzg2l_cpg_priv *priv)
> >>                 pd->id = info->pm_domains[i].id;
> >>                 pd->priv = priv;
> >>
> >> -               ret = rzg2l_cpg_pd_setup(pd, always_on);
> >> +               ret = rzg2l_cpg_pd_setup(pd, info->pm_domains[i].flags);
> >>                 if (ret)
> >>                         return ret;
> >>
> >> -               if (always_on) {
> >> +               if (info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON) {
> >>                         ret = rzg2l_cpg_power_on(&pd->genpd);
> >>                         if (ret)
> >>                                 return ret;
> >> @@ -1890,9 +1911,43 @@ static int __init rzg2l_cpg_probe(struct platform_device *pdev)
> >>         if (error)
> >>                 return error;
> >>
> >> +       dev_set_drvdata(dev, priv);
> >> +
> >>         return 0;
> >>  }
> >>
> >> +static int rzg2l_cpg_resume(struct device *dev)
> >> +{
> >> +       struct rzg2l_cpg_priv *priv = dev_get_drvdata(dev);
> >> +       const struct rzg2l_cpg_info *info = priv->info;
> >> +
> >> +       /* Power on always ON domains. */
> >> +       for (unsigned int i = 0; i < info->num_pm_domains; i++) {
> >> +               if (info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON) {
> >> +                       int ret = rzg2l_cpg_power_on(priv->domains[i]);
> >> +
> >> +                       if (ret)
> >> +                               return ret;
> >> +               }
> >> +       }
> >
> > I don't quite understand why this is needed? Is always-on PM domains
> > being powered-off during system wide suspend, so you need to power
> > them on again?
>
> Yes, as power to most of the system parts is cut off during sytem suspend
> (and DDR is kept in retention mode) and the resume is almost like a cold
> boot where the TF-A does basic re-initialization and then pass execution to
>  Linux resume code.

Hmm. If these are really always-on PM domains, why isn't the FW
powering them on again then before returning to Linux after a system
resume?

In a way it sounds to me that they aren't really always-on PM domains,
as Linux seems to be capable of turning them on/off too, right?

That said, perhaps using GENPD_FLAG_RPM_ALWAYS_ON instead of
GENPD_FLAG_ALWAYS_ON for some PM domains can be another way forward?
In this way, the ->power_on|off() callbacks can be used to turn on/off
the PM domains, but only during system suspend/resume. Would that
work?

[...]

Kind regards
Uffe

Re: [PATCH v2 08/10] clk: renesas: rzg2l-cpg: Add suspend/resume support for power domains

[claudiu beznea]

On 17.04.2024 12:39, Ulf Hansson wrote:
> On Wed, 17 Apr 2024 at 10:05, claudiu beznea <claudiu.beznea@tuxon.dev> wrote:
>>
>> Hi, Ulf,
>>
>> On 16.04.2024 15:07, Ulf Hansson wrote:
>>> On Thu, 7 Mar 2024 at 15:10, Claudiu <claudiu.beznea@tuxon.dev> wrote:
>>>>
>>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>>
>>>> RZ/G3S supports deep sleep states that it can reach with the help of the
>>>> TF-A.
>>>>
>>>> RZ/G3S has a few power domains (e.g. GIC) that need to be always-on while
>>>> Linux is running. These domains are initialized (and powered on) when
>>>> clock driver is probed.
>>>>
>>>> As the TF-A takes control at the very last(suspend)/first(resume)
>>>> phase of configuring the deep sleep state, it can do it's own settings on
>>>> power domains.
>>>
>>> For my understanding, can you please elaborate on this part a bit.
>>> What does the "last suspend/resume phase" mean, more exactly, here?
>>
>> The RZ/G3S SoC support a power saving mode where most of the SoC parts are
>> turned off and the system RAM is switched to retention mode. This is done
>> with the help of TF-A. The handshake b/w Linux and TF-A is done though the
>> drivers/firmware/psci/psci.c driver.
>>
>> After Linux finishes the execution of suspend code the control is taken by
>> TF-A. TF-A does the final settings on the system (e.g. switching the RAM to
>> retention mode) and power off most of the SoC parts.
>>
>> By the last phase of the suspend I'm referring to the TF-A doing the final
>> adjustments for the system to switch to this power saving mode.
>>
>> When resuming, as the TF-A is the 1st one being executed on the system
>> (this is what I called above the 1st phase of the resume), TF-A moves the
>> DDR out of retention mode, reconfigure basic IPs (like in boot case as most
>> of the SoC parts were powered off) and then give the control to Linux which
>> will execute the resume code.
> 
> Alright, thanks for clarifying! This makes sense to me now!
> 
>>
>>
>>>
>>>>
>>>> Thus, to restore the proper Linux state, add rzg2l_cpg_resume() which
>>>> powers on the always-on domains and rzg2l_cpg_complete() which activates
>>>> the power down mode for the IPs selected through CPG_PWRDN_IP{1, 2}.
>>>>
>>>> Along with it, added the suspend_check member to the RZ/G2L power domain
>>>> data structure whose purpose is to checks if a domain can be powered off
>>>> while the system is going to suspend. This is necessary for the serial
>>>> console domain which needs to be powered on if no_console_suspend is
>>>> available in bootargs.
>>>>
>>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>> ---

[ ... ]

>>>>
>>>> +static int rzg2l_cpg_resume(struct device *dev)
>>>> +{
>>>> +       struct rzg2l_cpg_priv *priv = dev_get_drvdata(dev);
>>>> +       const struct rzg2l_cpg_info *info = priv->info;
>>>> +
>>>> +       /* Power on always ON domains. */
>>>> +       for (unsigned int i = 0; i < info->num_pm_domains; i++) {
>>>> +               if (info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON) {
>>>> +                       int ret = rzg2l_cpg_power_on(priv->domains[i]);
>>>> +
>>>> +                       if (ret)
>>>> +                               return ret;
>>>> +               }
>>>> +       }
>>>
>>> I don't quite understand why this is needed? Is always-on PM domains
>>> being powered-off during system wide suspend, so you need to power
>>> them on again?
>>
>> Yes, as power to most of the system parts is cut off during sytem suspend
>> (and DDR is kept in retention mode) and the resume is almost like a cold
>> boot where the TF-A does basic re-initialization and then pass execution to
>>  Linux resume code.
> 
> Hmm. If these are really always-on PM domains, why isn't the FW
> powering them on again then before returning to Linux after a system
> resume?

I'll try to explain it better.

The power domain implementation in this series tries to abstract the
control of bus clock (though MSTOP registers) for individual modules
available in RZ/G3S SoC.

From hardware manual [1]: "The Module Standby Mode is a mode that requests
the clock stop of the module specified by the master. The purpose of this
mode is to reduce power consumption by stopping unnecessary functions."

MSTOP is connected to individual modules as described in this picture:
https://paste.pics/726c963c33a506651a4be5f327e2a46d

There is also the PWRDN functionality for the modules that are part of the
PD_ISOVCC domain. At the time of writing this series there was not much
information in hardware manual about PWRDN functionality. The design team
has been asked but there is no clear answer ATM if the sequence of using
PWRDN in Linux (as proposed in this series) is good or not. I encountered
no issues with it while experimenting thus I have kept it. If you prefer I
can drop it and return with something afterwards, if any.

As I said in the current implementation I also used the PWRDN. The PWRDN is
IP specific but takes effect (as of my experiments) when this is executed:

+       /* Prepare for power down the BUSes in power down mode. */
+       if (info->pm_domain_pwrdn_mstop)
+               writel(CPG_PWRDN_MSTOP_ENABLE, priv->base + CPG_PWRDN_MSTOP);

As of my experiments having CPG_PWRDN_MSTOP.CPG_PWRDN_MSTOP_ENABLE set
applies the PWRDN to all the IPs for which PWRDN bit has been set in
CPG_PWRDN_IP1 and CPG_PWRD_IP2 registers.

This settings are applied in probe after domains are powered (thus PWRDN
bits are properly set up for IP supporting it by calling ->power_on()) and
at the end of resume (after PWRDN bits are properly setup for IPs
supporting it by calling ->power_on())

From my experiments, when returning from suspend (thus after firmware has
been executed) the CPG_PWRDN_MSTOP_ENABLE bit at register CPG_PWRDN_MSTOP
is zero. We power on the domains in Linux after resuming because of PWRDN
functionality and CPG_PWRDN_MSTOP_ENABLE bit at register CPG_PWRDN_MSTOP.

> 
> In a way it sounds to me that they aren't really always-on PM domains,
> as Linux seems to be capable of turning them on/off too, right?

Yes, Linux has the ability of controlling them by setting MSTOP and PWRDN
bits. This is because the IP specific PWRDN functionality takes effect when
CPG_PWRDN_MSTOP_ENABLE bit at register CPG_PWRDN_MSTOP is set (as of my
experiments).

> 
> That said, perhaps using GENPD_FLAG_RPM_ALWAYS_ON instead of
> GENPD_FLAG_ALWAYS_ON for some PM domains can be another way forward?

All this is becuase PWRD functionality. Explaining it to you made me
consider that it would be better to just drop the PWRDN functionality at
the moment until it is fully understood. With it I think we should be able
to drop the rzg2l_cpg_power_on() in resume(), at least. What do you think?

> In this way, the ->power_on|off() callbacks can be used to turn on/off
> the PM domains, but only during system suspend/resume. Would that
> work?

I need to check it.

Thank you for you review,
Claudiu Beznea

[1]
https://www.renesas.com/us/en/products/microcontrollers-microprocessors/rz-mpus/rzg3s-general-purpose-microprocessors-single-core-arm-cortex-a55-11-ghz-cpu-and-dual-core-cortex-m33-250

> 
> [...]
> 
> Kind regards
> Uffe