[PATCH 00/12] soc: mediatek: pm-domains: Add new driver for SCPSYS power domains controller

[PATCH 00/12] soc: mediatek: pm-domains: Add new driver for SCPSYS power domains controller

[Enric Balletbo i Serra]

Dear all,

This is a new driver with the aim to deprecate the mtk-scpsys driver.
The problem with that driver is that, in order to support more Mediatek
SoCs you need to add some logic to handle properly the power-up
sequence of newer Mediatek SoCs, doesn't handle parent-child power
domains and need to hardcode all the clocks in the driver itself. The
result is that the driver is getting bigger and bigger every time a
new SoC needs to be supported.

All this information can be getted from a properly defined binding, so
can be cleaner and smaller, hence, we implemented a new driver. For
now, only MT8173 and MT8183 is supported but should be fairly easy to
add support for new SoCs.

Best regards,
  Enric

Enric Balletbo i Serra (4):
  dt-bindings: power: Add bindings for the Mediatek SCPSYS power domains
    controller
  soc: mediatek: Add MediaTek SCPSYS power domains
  arm64: dts: mediatek: Add mt8173 power domain controller
  dt-bindings: power: Add MT8183 power domains

Matthias Brugger (8):
  soc: mediatek: pm-domains: Add bus protection protocol
  soc: mediatek: pm_domains: Make bus protection generic
  soc: mediatek: pm-domains: Add SMI block as bus protection block
  soc: mediatek: pm-domains: Add extra sram control
  soc: mediatek: pm-domains: Add subsystem clocks
  soc: mediatek: pm-domains: Allow bus protection to ignore clear ack
  soc: mediatek: pm-domains: Add support for mt8183
  arm64: dts: mediatek: Add mt8183 power domains controller

 .../power/mediatek,power-controller.yaml      | 173 ++++
 arch/arm64/boot/dts/mediatek/mt8173.dtsi      |  78 +-
 arch/arm64/boot/dts/mediatek/mt8183.dtsi      | 160 +++
 drivers/soc/mediatek/Kconfig                  |  13 +
 drivers/soc/mediatek/Makefile                 |   1 +
 drivers/soc/mediatek/mtk-infracfg.c           |   5 -
 drivers/soc/mediatek/mtk-pm-domains.c         | 952 ++++++++++++++++++
 include/dt-bindings/power/mt8183-power.h      |  26 +
 include/linux/soc/mediatek/infracfg.h         |  39 +
 9 files changed, 1433 insertions(+), 14 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
 create mode 100644 drivers/soc/mediatek/mtk-pm-domains.c
 create mode 100644 include/dt-bindings/power/mt8183-power.h

-- 
2.28.0

[PATCH 01/12] dt-bindings: power: Add bindings for the Mediatek SCPSYS power domains controller

[Enric Balletbo i Serra]

The System Control Processor System (SCPSYS) has several power management
related tasks in the system. Add the bindings to define the power
domains for the SCPSYS power controller.

Co-developed-by: Matthias Brugger <mbrugger@suse.com>
Signed-off-by: Matthias Brugger <mbrugger@suse.com>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Dear Rob,

I am awasre that this binding is not ready, but I prefered to send because I'm
kind of blocked. Compiling this binding triggers the following error:

    mediatek,power-controller.example.dt.yaml: syscon@10006000: mfg_async@7:
    '#address-cells', '#size-cells', 'mfg_2d@8'
    do not match any of the regexes: 'pinctrl-[0-9]+'

This happens when a definition of a power-domain (parent) contains
another power-domain (child), like the example. I am not sure how to
specify this in the yaml and deal with this, so any clue is welcome.

Thanks,
  Enric

 .../power/mediatek,power-controller.yaml      | 171 ++++++++++++++++++
 1 file changed, 171 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/mediatek,power-controller.yaml

diff --git a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
new file mode 100644
index 000000000000..8be9244ad160
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
@@ -0,0 +1,171 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/mediatek,power-controller.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek Power Domains Controller
+
+maintainers:
+  - Weiyi Lu <weiyi.lu@mediatek.com>
+  - Matthias Brugger <mbrugger@suse.com>
+
+description: |
+  Mediatek processors include support for multiple power domains which can be
+  powered up/down by software based on different application scenes to save power.
+
+  IP cores belonging to a power domain should contain a 'power-domains'
+  property that is a phandle for SCPSYS node representing the domain.
+
+properties:
+  $nodename:
+    pattern: "^syscon@[0-9a-f]+$"
+
+  compatible:
+    items:
+      - enum:
+        - mediatek,mt8173-power-controller
+      - const: syscon
+
+  reg:
+    maxItems: 1
+
+patternProperties:
+  "^.*@[0-9]$":
+    type: object
+    description: |
+      Represents the power domains within the power controller node as documented
+      in Documentation/devicetree/bindings/power/power-domain.yaml.
+
+    properties:
+      reg:
+        description: |
+          Power domain index. Valid values are defined in:
+              "include/dt-bindings/power/mt8173-power.h" - for MT8173 type power domain.
+        maxItems: 1
+
+      '#power-domain-cells':
+        description:
+          Documented by the generic PM Domain bindings in
+          Documentation/devicetree/bindings/power/power-domain.yaml.
+
+      clocks:
+        description: |
+          A number of phandles to clocks that need to be enabled during domain
+          power-up sequencing.
+
+      clock-names:
+        description: |
+          List of names of clocks, in order to match the power-up sequencing
+          for each power domain we need to group the clocks by name. BASIC
+          clocks need to be enabled before enabling the corresponding power
+          domain, and should not have a '-' in their name (i.e mm, mfg, venc).
+          SUSBYS clocks need to be enabled before releasing the bus protection,
+          and should contain a '-' in their name (i.e mm-0, isp-0, cam-0).
+
+          In order to follow properly the power-up sequencing, the clocks must
+          be specified by order, adding first the BASIC clocks followed by the
+          SUSBSYS clocks.
+
+      mediatek,infracfg:
+        $ref: /schemas/types.yaml#definitions/phandle
+        description: phandle to the device containing the INFRACFG register range.
+
+      mediatek,smi:
+        $ref: /schemas/types.yaml#definitions/phandle
+        description: phandle to the device containing the SMI register range.
+
+    required:
+      - reg
+      - '#power-domain-cells'
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/mt8173-clk.h>
+    #include <dt-bindings/power/mt8173-power.h>
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        scpsys: syscon@10006000 {
+            compatible = "mediatek,mt8173-power-controller", "syscon";
+            reg = <0 0x10006000 0 0x1000>;
+
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            /* power domains of the SoC */
+            vdec@MT8173_POWER_DOMAIN_VDEC {
+                reg = <MT8173_POWER_DOMAIN_VDEC>;
+                clocks = <&topckgen CLK_TOP_MM_SEL>;
+                clock-names = "mm";
+                #power-domain-cells = <0>;
+            };
+
+            venc@MT8173_POWER_DOMAIN_VENC {
+                reg = <MT8173_POWER_DOMAIN_VENC>;
+                clocks = <&topckgen CLK_TOP_MM_SEL>,
+                         <&topckgen CLK_TOP_VENC_SEL>;
+                clock-names = "mm", "venc";
+                #power-domain-cells = <0>;
+            };
+            isp@MT8173_POWER_DOMAIN_ISP {
+                reg = <MT8173_POWER_DOMAIN_ISP>;
+                clocks = <&topckgen CLK_TOP_MM_SEL>;
+                clock-names = "mm";
+                #power-domain-cells = <0>;
+            };
+            mm@MT8173_POWER_DOMAIN_MM {
+                reg = <MT8173_POWER_DOMAIN_MM>;
+                clocks = <&topckgen CLK_TOP_MM_SEL>;
+                clock-names = "mm";
+                #power-domain-cells = <0>;
+                mediatek,infracfg = <&infracfg>;
+            };
+            venc_lt@MT8173_POWER_DOMAIN_VENC_LT {
+                reg = <MT8173_POWER_DOMAIN_VENC_LT>;
+                clocks = <&topckgen CLK_TOP_MM_SEL>,
+                         <&topckgen CLK_TOP_VENC_LT_SEL>;
+                clock-names = "mm", "venclt";
+                #power-domain-cells = <0>;
+            };
+            audio@MT8173_POWER_DOMAIN_AUDIO {
+                reg = <MT8173_POWER_DOMAIN_AUDIO>;
+                #power-domain-cells = <0>;
+            };
+            usb@MT8173_POWER_DOMAIN_USB {
+                reg = <MT8173_POWER_DOMAIN_USB>;
+                #power-domain-cells = <0>;
+            };
+            mfg_async@MT8173_POWER_DOMAIN_MFG_ASYNC {
+                reg = <MT8173_POWER_DOMAIN_MFG_ASYNC>;
+                clocks = <&clk26m>;
+                clock-names = "mfg";
+                #address-cells = <1>;
+                #size-cells = <0>;
+                #power-domain-cells = <1>;
+
+                mfg_2d@MT8173_POWER_DOMAIN_MFG_2D {
+                    reg = <MT8173_POWER_DOMAIN_MFG_2D>;
+                    #address-cells = <1>;
+                    #size-cells = <0>;
+                    #power-domain-cells = <1>;
+
+                    mfg@MT8173_POWER_DOMAIN_MFG {
+                        reg = <MT8173_POWER_DOMAIN_MFG>;
+                        #power-domain-cells = <0>;
+                        mediatek,infracfg = <&infracfg>;
+                    };
+                };
+            };
+        };
+    };
-- 
2.28.0

[PATCH 03/12] arm64: dts: mediatek: Add mt8173 power domain controller

[Enric Balletbo i Serra]

Add power domain controller node for SoC mt8173.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

 arch/arm64/boot/dts/mediatek/mt8173.dtsi | 78 +++++++++++++++++++++---
 1 file changed, 69 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index 5e046f9d48ce..3b08c5404d81 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -450,16 +450,76 @@ pins1 {
 			};
 		};
 
-		scpsys: power-controller@10006000 {
-			compatible = "mediatek,mt8173-scpsys";
-			#power-domain-cells = <1>;
+		scpsys: syscon@10006000 {
+			compatible = "mediatek,mt8173-power-controller";
 			reg = <0 0x10006000 0 0x1000>;
-			clocks = <&clk26m>,
-				 <&topckgen CLK_TOP_MM_SEL>,
-				 <&topckgen CLK_TOP_VENC_SEL>,
-				 <&topckgen CLK_TOP_VENC_LT_SEL>;
-			clock-names = "mfg", "mm", "venc", "venc_lt";
-			infracfg = <&infracfg>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			/* power domains of the SoC */
+			vdec@MT8173_POWER_DOMAIN_VDEC {
+				reg = <MT8173_POWER_DOMAIN_VDEC>;
+				clocks = <&topckgen CLK_TOP_MM_SEL>;
+				clock-names = "mm";
+				#power-domain-cells = <0>;
+			};
+
+			venc@MT8173_POWER_DOMAIN_VENC {
+				reg = <MT8173_POWER_DOMAIN_VENC>;
+				clocks = <&topckgen CLK_TOP_MM_SEL>,
+					 <&topckgen CLK_TOP_VENC_SEL>;
+				clock-names = "mm", "venc";
+				#power-domain-cells = <0>;
+			};
+			isp@MT8173_POWER_DOMAIN_ISP {
+				reg = <MT8173_POWER_DOMAIN_ISP>;
+				clocks = <&topckgen CLK_TOP_MM_SEL>;
+				clock-names = "mm";
+				#power-domain-cells = <0>;
+			};
+			mm@MT8173_POWER_DOMAIN_MM {
+				reg = <MT8173_POWER_DOMAIN_MM>;
+				clocks = <&topckgen CLK_TOP_MM_SEL>;
+				clock-names = "mm";
+				#power-domain-cells = <0>;
+				mediatek,infracfg = <&infracfg>;
+			};
+			venc_lt@MT8173_POWER_DOMAIN_VENC_LT {
+				reg = <MT8173_POWER_DOMAIN_VENC_LT>;
+				clocks = <&topckgen CLK_TOP_MM_SEL>,
+					 <&topckgen CLK_TOP_VENC_LT_SEL>;
+				clock-names = "mm", "venclt";
+				#power-domain-cells = <0>;
+			};
+			audio@MT8173_POWER_DOMAIN_AUDIO {
+				reg = <MT8173_POWER_DOMAIN_AUDIO>;
+				#power-domain-cells = <0>;
+			};
+			usb@MT8173_POWER_DOMAIN_USB {
+				reg = <MT8173_POWER_DOMAIN_USB>;
+				#power-domain-cells = <0>;
+			};
+			mfg_async@MT8173_POWER_DOMAIN_MFG_ASYNC {
+				reg = <MT8173_POWER_DOMAIN_MFG_ASYNC>;
+				clocks = <&clk26m>;
+				clock-names = "mfg";
+				#address-cells = <1>;
+				#size-cells = <0>;
+				#power-domain-cells = <1>;
+
+				mfg_2d@MT8173_POWER_DOMAIN_MFG_2D {
+					reg = <MT8173_POWER_DOMAIN_MFG_2D>;
+					#address-cells = <1>;
+					#size-cells = <0>;
+					#power-domain-cells = <1>;
+
+					mfg@MT8173_POWER_DOMAIN_MFG {
+						reg = <MT8173_POWER_DOMAIN_MFG>;
+						#power-domain-cells = <0>;
+						mediatek,infracfg = <&infracfg>;
+					};
+				};
+			};
 		};
 
 		watchdog: watchdog@10007000 {
-- 
2.28.0

[PATCH 10/12] dt-bindings: power: Add MT8183 power domains

[Enric Balletbo i Serra]

Add power domains dt-bindings for MT8183.

Signed-off-by: Matthias Brugger <mbrugger@suse.com>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

 .../power/mediatek,power-controller.yaml      |  2 ++
 include/dt-bindings/power/mt8183-power.h      | 26 +++++++++++++++++++
 2 files changed, 28 insertions(+)
 create mode 100644 include/dt-bindings/power/mt8183-power.h

diff --git a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
index 8be9244ad160..d828cae6f7db 100644
--- a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
+++ b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
@@ -25,6 +25,7 @@ properties:
     items:
       - enum:
         - mediatek,mt8173-power-controller
+        - mediatek,mt8183-power-controller
       - const: syscon
 
   reg:
@@ -42,6 +43,7 @@ patternProperties:
         description: |
           Power domain index. Valid values are defined in:
               "include/dt-bindings/power/mt8173-power.h" - for MT8173 type power domain.
+              "include/dt-bindings/power/mt8183-power.h" - for MT8183 type power domain.
         maxItems: 1
 
       '#power-domain-cells':
diff --git a/include/dt-bindings/power/mt8183-power.h b/include/dt-bindings/power/mt8183-power.h
new file mode 100644
index 000000000000..d1ab387ba8c7
--- /dev/null
+++ b/include/dt-bindings/power/mt8183-power.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2020 MediaTek Inc.
+ * Author: Weiyi Lu <weiyi.lu@mediatek.com>
+ */
+
+#ifndef _DT_BINDINGS_POWER_MT8183_POWER_H
+#define _DT_BINDINGS_POWER_MT8183_POWER_H
+
+#define MT8183_POWER_DOMAIN_AUDIO	0
+#define MT8183_POWER_DOMAIN_CONN	1
+#define MT8183_POWER_DOMAIN_MFG_ASYNC	2
+#define MT8183_POWER_DOMAIN_MFG		3
+#define MT8183_POWER_DOMAIN_MFG_CORE0	4
+#define MT8183_POWER_DOMAIN_MFG_CORE1	5
+#define MT8183_POWER_DOMAIN_MFG_2D	6
+#define MT8183_POWER_DOMAIN_DISP	7
+#define MT8183_POWER_DOMAIN_CAM		8
+#define MT8183_POWER_DOMAIN_ISP		9
+#define MT8183_POWER_DOMAIN_VDEC	10
+#define MT8183_POWER_DOMAIN_VENC	11
+#define MT8183_POWER_DOMAIN_VPU_TOP	12
+#define MT8183_POWER_DOMAIN_VPU_CORE0	13
+#define MT8183_POWER_DOMAIN_VPU_CORE1	14
+
+#endif /* _DT_BINDINGS_POWER_MT8183_POWER_H */
-- 
2.28.0

[PATCH 12/12] arm64: dts: mediatek: Add mt8183 power domains controller

[Enric Balletbo i Serra]

From: Matthias Brugger <mbrugger@suse.com>

Add power domains controller node for SoC mt8183

Signed-off-by: Matthias Brugger <mbrugger@suse.com>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 160 +++++++++++++++++++++++
 1 file changed, 160 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 102105871db2..7012cdb22bf0 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -8,6 +8,7 @@
 #include <dt-bindings/clock/mt8183-clk.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/power/mt8183-power.h>
 #include <dt-bindings/reset-controller/mt8183-resets.h>
 #include <dt-bindings/phy/phy.h>
 #include "mt8183-pinfunc.h"
@@ -316,6 +317,160 @@ pio: pinctrl@10005000 {
 			#interrupt-cells = <2>;
 		};
 
+		scpsys: syscon@10006000 {
+			compatible = "mediatek,mt8183-power-controller", "syscon";
+			reg = <0 0x10006000 0 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			audio@MT8183_POWER_DOMAIN_AUDIO {
+				reg = <MT8183_POWER_DOMAIN_AUDIO>;
+				clocks = <&topckgen CLK_TOP_MUX_AUD_INTBUS>,
+					 <&infracfg CLK_INFRA_AUDIO>,
+					 <&infracfg CLK_INFRA_AUDIO_26M_BCLK>;
+				clock-names = "audio", "audio1", "audio2";
+				#power-domain-cells = <0>;
+			};
+
+			conn@MT8183_POWER_DOMAIN_CONN {
+				reg = <MT8183_POWER_DOMAIN_CONN>;
+				mediatek,infracfg = <&infracfg>;
+				#power-domain-cells = <0>;
+			};
+
+			mfg_async@MT8183_POWER_DOMAIN_MFG_ASYNC {
+				reg = <MT8183_POWER_DOMAIN_MFG_ASYNC>;
+
+				#address-cells = <1>;
+				#size-cells = <0>;
+				clocks =  <&topckgen CLK_TOP_MUX_MFG>;
+				clock-names = "mfg";
+				#power-domain-cells = <1>;
+
+				mfg@MT8183_POWER_DOMAIN_MFG {
+					reg = <MT8183_POWER_DOMAIN_MFG>;
+					#address-cells = <1>;
+					#size-cells = <0>;
+					#power-domain-cells = <1>;
+
+					mfg_core0@MT8183_POWER_DOMAIN_MFG_CORE0 {
+						reg = <MT8183_POWER_DOMAIN_MFG_CORE0>;
+						#power-domain-cells = <0>;
+					};
+
+					mfg_core1@MT8183_POWER_DOMAIN_MFG_CORE1 {
+						reg = <MT8183_POWER_DOMAIN_MFG_CORE1>;
+						#power-domain-cells = <0>;
+					};
+
+					mfg_2d@MT8183_POWER_DOMAIN_MFG_2D {
+						reg = <MT8183_POWER_DOMAIN_MFG_2D>;
+						mediatek,infracfg = <&infracfg>;
+						#power-domain-cells = <0>;
+					};
+				};
+			};
+
+			disp@MT8183_POWER_DOMAIN_DISP {
+				reg = <MT8183_POWER_DOMAIN_DISP>;
+				clocks = <&topckgen CLK_TOP_MUX_MM>,
+					 <&mmsys CLK_MM_SMI_COMMON>,
+					 <&mmsys CLK_MM_SMI_LARB0>,
+					 <&mmsys CLK_MM_SMI_LARB1>,
+					 <&mmsys CLK_MM_GALS_COMM0>,
+					 <&mmsys CLK_MM_GALS_COMM1>,
+					 <&mmsys CLK_MM_GALS_CCU2MM>,
+					 <&mmsys CLK_MM_GALS_IPU12MM>,
+					 <&mmsys CLK_MM_GALS_IMG2MM>,
+					 <&mmsys CLK_MM_GALS_CAM2MM>,
+					 <&mmsys CLK_MM_GALS_IPU2MM>;
+				clock-names = "mm", "mm-0", "mm-1", "mm-2", "mm-3",
+					      "mm-4", "mm-5", "mm-6", "mm-7",
+					      "mm-8", "mm-9";
+				#address-cells = <1>;
+				#size-cells = <0>;
+				mediatek,infracfg = <&infracfg>;
+				mediatek,smi = <&smi_common>;
+				#power-domain-cells = <1>;
+
+				cam@MT8183_POWER_DOMAIN_CAM {
+					reg = <MT8183_POWER_DOMAIN_CAM>;
+					clocks = <&topckgen CLK_TOP_MUX_CAM>,
+						 <&camsys CLK_CAM_LARB6>,
+						 <&camsys CLK_CAM_LARB3>,
+						 <&camsys CLK_CAM_SENINF>,
+						 <&camsys CLK_CAM_CAMSV0>,
+						 <&camsys CLK_CAM_CAMSV1>,
+						 <&camsys CLK_CAM_CAMSV2>,
+						 <&camsys CLK_CAM_CCU>;
+					clock-names = "cam", "cam-0", "cam-1",
+						      "cam-2", "cam-3", "cam-4",
+						      "cam-5", "cam-6";
+					mediatek,infracfg = <&infracfg>;
+					mediatek,smi = <&smi_common>;
+					#power-domain-cells = <0>;
+				};
+
+				isp@MT8183_POWER_DOMAIN_ISP {
+					reg = <MT8183_POWER_DOMAIN_ISP>;
+					clocks = <&topckgen CLK_TOP_MUX_IMG>,
+						 <&imgsys CLK_IMG_LARB5>,
+						 <&imgsys CLK_IMG_LARB2>;
+					clock-names = "isp", "isp-0", "isp-1";
+					mediatek,infracfg = <&infracfg>;
+					mediatek,smi = <&smi_common>;
+					#power-domain-cells = <0>;
+				};
+
+				vdec@MT8183_POWER_DOMAIN_VDEC {
+					reg = <MT8183_POWER_DOMAIN_VDEC>;
+					mediatek,smi = <&smi_common>;
+					#power-domain-cells = <0>;
+				};
+
+				venc@MT8183_POWER_DOMAIN_VENC {
+					reg = <MT8183_POWER_DOMAIN_VENC>;
+					mediatek,smi = <&smi_common>;
+					#power-domain-cells = <0>;
+				};
+
+				vpu_top@MT8183_POWER_DOMAIN_VPU_TOP {
+					reg = <MT8183_POWER_DOMAIN_VPU_TOP>;
+					clocks = <&topckgen CLK_TOP_MUX_IPU_IF>,
+						 <&topckgen CLK_TOP_MUX_DSP>,
+						 <&ipu_conn CLK_IPU_CONN_IPU>,
+						 <&ipu_conn CLK_IPU_CONN_AHB>,
+						 <&ipu_conn CLK_IPU_CONN_AXI>,
+						 <&ipu_conn CLK_IPU_CONN_ISP>,
+						 <&ipu_conn CLK_IPU_CONN_CAM_ADL>,
+						 <&ipu_conn CLK_IPU_CONN_IMG_ADL>;
+					clock-names = "vpu", "vpu1", "vpu-0", "vpu-1",
+						      "vpu-2", "vpu-3", "vpu-4", "vpu-5";
+					#address-cells = <1>;
+					#size-cells = <0>;
+					mediatek,infracfg = <&infracfg>;
+					mediatek,smi = <&smi_common>;
+					#power-domain-cells = <1>;
+
+					vpu_core0@MT8183_POWER_DOMAIN_VPU_CORE0 {
+						reg = <MT8183_POWER_DOMAIN_VPU_CORE0>;
+						clocks = <&topckgen CLK_TOP_MUX_DSP1>;
+						clock-names = "vpu2";
+						mediatek,infracfg = <&infracfg>;
+						#power-domain-cells = <0>;
+					};
+
+					vpu_core1@MT8183_POWER_DOMAIN_VPU_CORE1 {
+						reg = <MT8183_POWER_DOMAIN_VPU_CORE1>;
+						clocks = <&topckgen CLK_TOP_MUX_DSP2>;
+						clock-names = "vpu3";
+						mediatek,infracfg = <&infracfg>;
+						#power-domain-cells = <0>;
+					};
+				};
+			};
+		};
+
 		watchdog: watchdog@10007000 {
 			compatible = "mediatek,mt8183-wdt",
 				     "mediatek,mt6589-wdt";
@@ -754,6 +909,11 @@ mmsys: syscon@14000000 {
 			#clock-cells = <1>;
 		};
 
+		smi_common: smi@14019000 {
+			compatible = "mediatek,mt8183-smi-common", "syscon";
+			reg = <0 0x14019000 0 0x1000>;
+		};
+
 		imgsys: syscon@15020000 {
 			compatible = "mediatek,mt8183-imgsys", "syscon";
 			reg = <0 0x15020000 0 0x1000>;
-- 
2.28.0

Re: [PATCH 01/12] dt-bindings: power: Add bindings for the Mediatek SCPSYS power domains controller

[Rob Herring]

On Thu, Sep 10, 2020 at 07:28:15PM +0200, Enric Balletbo i Serra wrote:
> The System Control Processor System (SCPSYS) has several power management
> related tasks in the system. Add the bindings to define the power
> domains for the SCPSYS power controller.
> 
> Co-developed-by: Matthias Brugger <mbrugger@suse.com>
> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> Dear Rob,
> 
> I am awasre that this binding is not ready, but I prefered to send because I'm
> kind of blocked. Compiling this binding triggers the following error:
> 
>     mediatek,power-controller.example.dt.yaml: syscon@10006000: mfg_async@7:
>     '#address-cells', '#size-cells', 'mfg_2d@8'
>     do not match any of the regexes: 'pinctrl-[0-9]+'
> 
> This happens when a definition of a power-domain (parent) contains
> another power-domain (child), like the example. I am not sure how to
> specify this in the yaml and deal with this, so any clue is welcome.

You just have to keep nesting schemas all the way down. Define a 
grandchild node under the child node and then all of its properties.

> 
> Thanks,
>   Enric
> 
>  .../power/mediatek,power-controller.yaml      | 171 ++++++++++++++++++
>  1 file changed, 171 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> 
> diff --git a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> new file mode 100644
> index 000000000000..8be9244ad160
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> @@ -0,0 +1,171 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/mediatek,power-controller.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Mediatek Power Domains Controller
> +
> +maintainers:
> +  - Weiyi Lu <weiyi.lu@mediatek.com>
> +  - Matthias Brugger <mbrugger@suse.com>
> +
> +description: |
> +  Mediatek processors include support for multiple power domains which can be
> +  powered up/down by software based on different application scenes to save power.
> +
> +  IP cores belonging to a power domain should contain a 'power-domains'
> +  property that is a phandle for SCPSYS node representing the domain.
> +
> +properties:
> +  $nodename:
> +    pattern: "^syscon@[0-9a-f]+$"
> +
> +  compatible:
> +    items:
> +      - enum:
> +        - mediatek,mt8173-power-controller
> +      - const: syscon
> +
> +  reg:
> +    maxItems: 1
> +
> +patternProperties:
> +  "^.*@[0-9]$":

Node names should be generic:

power-domain@

> +    type: object
> +    description: |
> +      Represents the power domains within the power controller node as documented
> +      in Documentation/devicetree/bindings/power/power-domain.yaml.
> +
> +    properties:
> +      reg:
> +        description: |
> +          Power domain index. Valid values are defined in:
> +              "include/dt-bindings/power/mt8173-power.h" - for MT8173 type power domain.
> +        maxItems: 1
> +
> +      '#power-domain-cells':
> +        description:
> +          Documented by the generic PM Domain bindings in
> +          Documentation/devicetree/bindings/power/power-domain.yaml.

No need to redefine a common property. This should define valid values 
for it.

> +
> +      clocks:
> +        description: |
> +          A number of phandles to clocks that need to be enabled during domain
> +          power-up sequencing.

No need to redefine 'clocks'. You need to define how many, what each one 
is, and the order.

> +
> +      clock-names:
> +        description: |
> +          List of names of clocks, in order to match the power-up sequencing
> +          for each power domain we need to group the clocks by name. BASIC
> +          clocks need to be enabled before enabling the corresponding power
> +          domain, and should not have a '-' in their name (i.e mm, mfg, venc).
> +          SUSBYS clocks need to be enabled before releasing the bus protection,
> +          and should contain a '-' in their name (i.e mm-0, isp-0, cam-0).
> +
> +          In order to follow properly the power-up sequencing, the clocks must
> +          be specified by order, adding first the BASIC clocks followed by the
> +          SUSBSYS clocks.

You need to define the names.

> +
> +      mediatek,infracfg:
> +        $ref: /schemas/types.yaml#definitions/phandle
> +        description: phandle to the device containing the INFRACFG register range.
> +
> +      mediatek,smi:
> +        $ref: /schemas/types.yaml#definitions/phandle
> +        description: phandle to the device containing the SMI register range.
> +
> +    required:
> +      - reg
> +      - '#power-domain-cells'
> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/mt8173-clk.h>
> +    #include <dt-bindings/power/mt8173-power.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        scpsys: syscon@10006000 {
> +            compatible = "mediatek,mt8173-power-controller", "syscon";
> +            reg = <0 0x10006000 0 0x1000>;
> +
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            /* power domains of the SoC */
> +            vdec@MT8173_POWER_DOMAIN_VDEC {
> +                reg = <MT8173_POWER_DOMAIN_VDEC>;
> +                clocks = <&topckgen CLK_TOP_MM_SEL>;
> +                clock-names = "mm";
> +                #power-domain-cells = <0>;
> +            };
> +
> +            venc@MT8173_POWER_DOMAIN_VENC {
> +                reg = <MT8173_POWER_DOMAIN_VENC>;
> +                clocks = <&topckgen CLK_TOP_MM_SEL>,
> +                         <&topckgen CLK_TOP_VENC_SEL>;
> +                clock-names = "mm", "venc";
> +                #power-domain-cells = <0>;
> +            };
> +            isp@MT8173_POWER_DOMAIN_ISP {
> +                reg = <MT8173_POWER_DOMAIN_ISP>;
> +                clocks = <&topckgen CLK_TOP_MM_SEL>;
> +                clock-names = "mm";
> +                #power-domain-cells = <0>;
> +            };
> +            mm@MT8173_POWER_DOMAIN_MM {
> +                reg = <MT8173_POWER_DOMAIN_MM>;
> +                clocks = <&topckgen CLK_TOP_MM_SEL>;
> +                clock-names = "mm";
> +                #power-domain-cells = <0>;
> +                mediatek,infracfg = <&infracfg>;
> +            };
> +            venc_lt@MT8173_POWER_DOMAIN_VENC_LT {
> +                reg = <MT8173_POWER_DOMAIN_VENC_LT>;
> +                clocks = <&topckgen CLK_TOP_MM_SEL>,
> +                         <&topckgen CLK_TOP_VENC_LT_SEL>;
> +                clock-names = "mm", "venclt";
> +                #power-domain-cells = <0>;
> +            };
> +            audio@MT8173_POWER_DOMAIN_AUDIO {
> +                reg = <MT8173_POWER_DOMAIN_AUDIO>;
> +                #power-domain-cells = <0>;
> +            };
> +            usb@MT8173_POWER_DOMAIN_USB {
> +                reg = <MT8173_POWER_DOMAIN_USB>;
> +                #power-domain-cells = <0>;
> +            };
> +            mfg_async@MT8173_POWER_DOMAIN_MFG_ASYNC {
> +                reg = <MT8173_POWER_DOMAIN_MFG_ASYNC>;
> +                clocks = <&clk26m>;
> +                clock-names = "mfg";
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +                #power-domain-cells = <1>;
> +
> +                mfg_2d@MT8173_POWER_DOMAIN_MFG_2D {
> +                    reg = <MT8173_POWER_DOMAIN_MFG_2D>;
> +                    #address-cells = <1>;
> +                    #size-cells = <0>;
> +                    #power-domain-cells = <1>;
> +
> +                    mfg@MT8173_POWER_DOMAIN_MFG {
> +                        reg = <MT8173_POWER_DOMAIN_MFG>;
> +                        #power-domain-cells = <0>;
> +                        mediatek,infracfg = <&infracfg>;
> +                    };
> +                };
> +            };
> +        };
> +    };
> -- 
> 2.28.0
> 

Re: [PATCH 01/12] dt-bindings: power: Add bindings for the Mediatek SCPSYS power domains controller

[Matthias Brugger]

On 12/09/2020 01:02, Rob Herring wrote:
> On Thu, Sep 10, 2020 at 07:28:15PM +0200, Enric Balletbo i Serra wrote:
>> The System Control Processor System (SCPSYS) has several power management
>> related tasks in the system. Add the bindings to define the power
>> domains for the SCPSYS power controller.
>>
>> Co-developed-by: Matthias Brugger <mbrugger@suse.com>
>> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> ---
>> Dear Rob,
>>
>> I am awasre that this binding is not ready, but I prefered to send because I'm
>> kind of blocked. Compiling this binding triggers the following error:
>>
>>      mediatek,power-controller.example.dt.yaml: syscon@10006000: mfg_async@7:
>>      '#address-cells', '#size-cells', 'mfg_2d@8'
>>      do not match any of the regexes: 'pinctrl-[0-9]+'
>>
>> This happens when a definition of a power-domain (parent) contains
>> another power-domain (child), like the example. I am not sure how to
>> specify this in the yaml and deal with this, so any clue is welcome.
> 
> You just have to keep nesting schemas all the way down. Define a
> grandchild node under the child node and then all of its properties.
> 
>>
>> Thanks,
>>    Enric
>>
>>   .../power/mediatek,power-controller.yaml      | 171 ++++++++++++++++++
>>   1 file changed, 171 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
>> new file mode 100644
>> index 000000000000..8be9244ad160
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
>> @@ -0,0 +1,171 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/power/mediatek,power-controller.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Mediatek Power Domains Controller
>> +
>> +maintainers:
>> +  - Weiyi Lu <weiyi.lu@mediatek.com>
>> +  - Matthias Brugger <mbrugger@suse.com>
>> +
>> +description: |
>> +  Mediatek processors include support for multiple power domains which can be
>> +  powered up/down by software based on different application scenes to save power.
>> +
>> +  IP cores belonging to a power domain should contain a 'power-domains'
>> +  property that is a phandle for SCPSYS node representing the domain.
>> +
>> +properties:
>> +  $nodename:
>> +    pattern: "^syscon@[0-9a-f]+$"
>> +
>> +  compatible:
>> +    items:
>> +      - enum:
>> +        - mediatek,mt8173-power-controller
>> +      - const: syscon
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +patternProperties:
>> +  "^.*@[0-9]$":
> 
> Node names should be generic:
> 
> power-domain@
> 

Enric correct me if I'm wrong, if we want to see the power domains in debugfs, 
they are listed by their name. If all are called power-domain then the listing 
is pretty much useless.

>> +    type: object
>> +    description: |
>> +      Represents the power domains within the power controller node as documented
>> +      in Documentation/devicetree/bindings/power/power-domain.yaml.
>> +
>> +    properties:
>> +      reg:
>> +        description: |
>> +          Power domain index. Valid values are defined in:
>> +              "include/dt-bindings/power/mt8173-power.h" - for MT8173 type power domain.
>> +        maxItems: 1
>> +
>> +      '#power-domain-cells':
>> +        description:
>> +          Documented by the generic PM Domain bindings in
>> +          Documentation/devicetree/bindings/power/power-domain.yaml.
> 
> No need to redefine a common property. This should define valid values
> for it.
> 
>> +
>> +      clocks:
>> +        description: |
>> +          A number of phandles to clocks that need to be enabled during domain
>> +          power-up sequencing.
> 
> No need to redefine 'clocks'. You need to define how many, what each one
> is, and the order.
> 

Do you mean we have to define each clock for each power domain of each SoC?

>> +
>> +      clock-names:
>> +        description: |
>> +          List of names of clocks, in order to match the power-up sequencing
>> +          for each power domain we need to group the clocks by name. BASIC
>> +          clocks need to be enabled before enabling the corresponding power
>> +          domain, and should not have a '-' in their name (i.e mm, mfg, venc).
>> +          SUSBYS clocks need to be enabled before releasing the bus protection,
>> +          and should contain a '-' in their name (i.e mm-0, isp-0, cam-0).
>> +
>> +          In order to follow properly the power-up sequencing, the clocks must
>> +          be specified by order, adding first the BASIC clocks followed by the
>> +          SUSBSYS clocks.
> 
> You need to define the names.
> 
>> +
>> +      mediatek,infracfg:
>> +        $ref: /schemas/types.yaml#definitions/phandle
>> +        description: phandle to the device containing the INFRACFG register range.
>> +
>> +      mediatek,smi:
>> +        $ref: /schemas/types.yaml#definitions/phandle
>> +        description: phandle to the device containing the SMI register range.
>> +
>> +    required:
>> +      - reg
>> +      - '#power-domain-cells'
>> +
>> +    additionalProperties: false
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/mt8173-clk.h>
>> +    #include <dt-bindings/power/mt8173-power.h>
>> +
>> +    soc {
>> +        #address-cells = <2>;
>> +        #size-cells = <2>;
>> +
>> +        scpsys: syscon@10006000 {
>> +            compatible = "mediatek,mt8173-power-controller", "syscon";

The power domain controller is just one funcionality the SCPSYS block can 
provide. I think it should be child of the SCPSYS.

Regards,
Matthias

Re: [PATCH 01/12] dt-bindings: power: Add bindings for the Mediatek SCPSYS power domains controller

[Enric Balletbo i Serra]

On 14/9/20 10:59, Matthias Brugger wrote:
> 
> 
> On 12/09/2020 01:02, Rob Herring wrote:
>> On Thu, Sep 10, 2020 at 07:28:15PM +0200, Enric Balletbo i Serra wrote:
>>> The System Control Processor System (SCPSYS) has several power management
>>> related tasks in the system. Add the bindings to define the power
>>> domains for the SCPSYS power controller.
>>>
>>> Co-developed-by: Matthias Brugger <mbrugger@suse.com>
>>> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>>> ---
>>> Dear Rob,
>>>
>>> I am awasre that this binding is not ready, but I prefered to send because I'm
>>> kind of blocked. Compiling this binding triggers the following error:
>>>
>>>      mediatek,power-controller.example.dt.yaml: syscon@10006000: mfg_async@7:
>>>      '#address-cells', '#size-cells', 'mfg_2d@8'
>>>      do not match any of the regexes: 'pinctrl-[0-9]+'
>>>
>>> This happens when a definition of a power-domain (parent) contains
>>> another power-domain (child), like the example. I am not sure how to
>>> specify this in the yaml and deal with this, so any clue is welcome.
>>
>> You just have to keep nesting schemas all the way down. Define a
>> grandchild node under the child node and then all of its properties.
>>
>>>
>>> Thanks,
>>>    Enric
>>>
>>>   .../power/mediatek,power-controller.yaml      | 171 ++++++++++++++++++
>>>   1 file changed, 171 insertions(+)
>>>   create mode 100644
>>> Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
>>> b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
>>> new file mode 100644
>>> index 000000000000..8be9244ad160
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
>>> @@ -0,0 +1,171 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/power/mediatek,power-controller.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Mediatek Power Domains Controller
>>> +
>>> +maintainers:
>>> +  - Weiyi Lu <weiyi.lu@mediatek.com>
>>> +  - Matthias Brugger <mbrugger@suse.com>
>>> +
>>> +description: |
>>> +  Mediatek processors include support for multiple power domains which can be
>>> +  powered up/down by software based on different application scenes to save
>>> power.
>>> +
>>> +  IP cores belonging to a power domain should contain a 'power-domains'
>>> +  property that is a phandle for SCPSYS node representing the domain.
>>> +
>>> +properties:
>>> +  $nodename:
>>> +    pattern: "^syscon@[0-9a-f]+$"
>>> +
>>> +  compatible:
>>> +    items:
>>> +      - enum:
>>> +        - mediatek,mt8173-power-controller
>>> +      - const: syscon
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +patternProperties:
>>> +  "^.*@[0-9]$":
>>
>> Node names should be generic:
>>
>> power-domain@
>>
> 
> Enric correct me if I'm wrong, if we want to see the power domains in debugfs,
> they are listed by their name. If all are called power-domain then the listing
> is pretty much useless.
> 

cc'ing Dafna who might be interested in this discussion.

Yes, It'd be difficult to clearly identify which domain is without looking at
the DT. Now we have

# ls /sys/kernel/debug/pm_genpd
audio  mfg     mfg_async  pm_genpd_summary  vdec  venc_lt
isp    mfg_2d  mm         usb


Actually, I see two "problems" on using a generic name. The first one is that
debugfs uses that name and doesn't allow duplicate names, so we will get a bunch
of errors like this:

debugfs: Directory 'power-domain' with parent 'pm_gendpd' already present!
debugfs: Directory 'power-domain' with parent 'pm_gendpd' already present!
debugfs: Directory 'power-domain' with parent 'pm_gendpd' already present!
...

And we will lost the debug information. However, that's not probably a DT
problem as maybe debugfs should create different names in the form
power-domain@0, power-domain@1, etc.

The second one is what Matthias said, the name exported to the debugfs is
useless. Again, maybe is not a DT problem and the debugfs infra should handle
this cases in a better way, but that's not the case right now.


>>> +    type: object
>>> +    description: |
>>> +      Represents the power domains within the power controller node as
>>> documented
>>> +      in Documentation/devicetree/bindings/power/power-domain.yaml.
>>> +
>>> +    properties:
>>> +      reg:
>>> +        description: |
>>> +          Power domain index. Valid values are defined in:
>>> +              "include/dt-bindings/power/mt8173-power.h" - for MT8173 type
>>> power domain.
>>> +        maxItems: 1
>>> +
>>> +      '#power-domain-cells':
>>> +        description:
>>> +          Documented by the generic PM Domain bindings in
>>> +          Documentation/devicetree/bindings/power/power-domain.yaml.
>>
>> No need to redefine a common property. This should define valid values
>> for it.
>>
>>> +
>>> +      clocks:
>>> +        description: |
>>> +          A number of phandles to clocks that need to be enabled during domain
>>> +          power-up sequencing.
>>
>> No need to redefine 'clocks'. You need to define how many, what each one
>> is, and the order.
>>
> 
> Do you mean we have to define each clock for each power domain of each SoC?
> 
>>> +
>>> +      clock-names:
>>> +        description: |
>>> +          List of names of clocks, in order to match the power-up sequencing
>>> +          for each power domain we need to group the clocks by name. BASIC
>>> +          clocks need to be enabled before enabling the corresponding power
>>> +          domain, and should not have a '-' in their name (i.e mm, mfg, venc).
>>> +          SUSBYS clocks need to be enabled before releasing the bus protection,
>>> +          and should contain a '-' in their name (i.e mm-0, isp-0, cam-0).
>>> +
>>> +          In order to follow properly the power-up sequencing, the clocks must
>>> +          be specified by order, adding first the BASIC clocks followed by the
>>> +          SUSBSYS clocks.
>>
>> You need to define the names.
>>
>>> +
>>> +      mediatek,infracfg:
>>> +        $ref: /schemas/types.yaml#definitions/phandle
>>> +        description: phandle to the device containing the INFRACFG register
>>> range.
>>> +
>>> +      mediatek,smi:
>>> +        $ref: /schemas/types.yaml#definitions/phandle
>>> +        description: phandle to the device containing the SMI register range.
>>> +
>>> +    required:
>>> +      - reg
>>> +      - '#power-domain-cells'
>>> +
>>> +    additionalProperties: false
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/clock/mt8173-clk.h>
>>> +    #include <dt-bindings/power/mt8173-power.h>
>>> +
>>> +    soc {
>>> +        #address-cells = <2>;
>>> +        #size-cells = <2>;
>>> +
>>> +        scpsys: syscon@10006000 {
>>> +            compatible = "mediatek,mt8173-power-controller", "syscon";
> 
> The power domain controller is just one funcionality the SCPSYS block can
> provide. I think it should be child of the SCPSYS.
> 
> Regards,
> Matthias
> 

Re: [PATCH 03/12] arm64: dts: mediatek: Add mt8173 power domain controller

[Fabien Parent]

Hi Enric,

> -               scpsys: power-controller@10006000 {
> -                       compatible = "mediatek,mt8173-scpsys";
> -                       #power-domain-cells = <1>;

This change generates a lot of warning when compiling the MT8173 device-trees.

Warning (power_domains_property): /soc/mutex@14020000: Missing
property '#power-domain-cells' in node /soc/syscon@10006000 or bad
phandle (referred from power-domains[0])

Re: [PATCH 03/12] arm64: dts: mediatek: Add mt8173 power domain controller

[Enric Balletbo Serra]

Hi Fabien,

Thank you to look at this.

Missatge de Fabien Parent <fparent@baylibre.com> del dia dv., 18 de
set. 2020 a les 22:24:
>
> Hi Enric,
>
> > -               scpsys: power-controller@10006000 {
> > -                       compatible = "mediatek,mt8173-scpsys";
> > -                       #power-domain-cells = <1>;
>
> This change generates a lot of warning when compiling the MT8173 device-trees.
>
> Warning (power_domains_property): /soc/mutex@14020000: Missing
> property '#power-domain-cells' in node /soc/syscon@10006000 or bad
> phandle (referred from power-domains[0])

I think that there is a mistake in that patch #power-domain-cells =
<1>; should not be removed. Anyway, I talked with Matthias and I'm
going to redefine this part as doesn't really match with the hardware.
We're thinking on something like this:

scpsys: syscon@10006000 {
     compatible = "mediatek,mtk-scpsys", "syscon";
      reg = ...

     power-controller {
           compatible = "mediatek,mt8173-power-controller";
           #power-domain-cells = <1>;

           <- the list of domains ->

Thanks,
  Enric

Re: [PATCH 01/12] dt-bindings: power: Add bindings for the Mediatek SCPSYS power domains controller

[Rob Herring]

On Mon, Sep 14, 2020 at 2:59 AM Matthias Brugger <matthias.bgg@gmail.com> wrote:
>
>
>
> On 12/09/2020 01:02, Rob Herring wrote:
> > On Thu, Sep 10, 2020 at 07:28:15PM +0200, Enric Balletbo i Serra wrote:
> >> The System Control Processor System (SCPSYS) has several power management
> >> related tasks in the system. Add the bindings to define the power
> >> domains for the SCPSYS power controller.
> >>
> >> Co-developed-by: Matthias Brugger <mbrugger@suse.com>
> >> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
> >> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> >> ---
> >> Dear Rob,
> >>
> >> I am awasre that this binding is not ready, but I prefered to send because I'm
> >> kind of blocked. Compiling this binding triggers the following error:
> >>
> >>      mediatek,power-controller.example.dt.yaml: syscon@10006000: mfg_async@7:
> >>      '#address-cells', '#size-cells', 'mfg_2d@8'
> >>      do not match any of the regexes: 'pinctrl-[0-9]+'
> >>
> >> This happens when a definition of a power-domain (parent) contains
> >> another power-domain (child), like the example. I am not sure how to
> >> specify this in the yaml and deal with this, so any clue is welcome.
> >
> > You just have to keep nesting schemas all the way down. Define a
> > grandchild node under the child node and then all of its properties.
> >
> >>
> >> Thanks,
> >>    Enric
> >>
> >>   .../power/mediatek,power-controller.yaml      | 171 ++++++++++++++++++
> >>   1 file changed, 171 insertions(+)
> >>   create mode 100644 Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> >>
> >> diff --git a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> >> new file mode 100644
> >> index 000000000000..8be9244ad160
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> >> @@ -0,0 +1,171 @@
> >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/power/mediatek,power-controller.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: Mediatek Power Domains Controller
> >> +
> >> +maintainers:
> >> +  - Weiyi Lu <weiyi.lu@mediatek.com>
> >> +  - Matthias Brugger <mbrugger@suse.com>
> >> +
> >> +description: |
> >> +  Mediatek processors include support for multiple power domains which can be
> >> +  powered up/down by software based on different application scenes to save power.
> >> +
> >> +  IP cores belonging to a power domain should contain a 'power-domains'
> >> +  property that is a phandle for SCPSYS node representing the domain.
> >> +
> >> +properties:
> >> +  $nodename:
> >> +    pattern: "^syscon@[0-9a-f]+$"
> >> +
> >> +  compatible:
> >> +    items:
> >> +      - enum:
> >> +        - mediatek,mt8173-power-controller
> >> +      - const: syscon
> >> +
> >> +  reg:
> >> +    maxItems: 1
> >> +
> >> +patternProperties:
> >> +  "^.*@[0-9]$":
> >
> > Node names should be generic:
> >
> > power-domain@
> >
>
> Enric correct me if I'm wrong, if we want to see the power domains in debugfs,
> they are listed by their name. If all are called power-domain then the listing
> is pretty much useless.

Sorry, but not a binding problem.

Maybe if debugfs shows what devices are contained within a power
domain then it doesn't matter so much.

> >> +    type: object
> >> +    description: |
> >> +      Represents the power domains within the power controller node as documented
> >> +      in Documentation/devicetree/bindings/power/power-domain.yaml.
> >> +
> >> +    properties:
> >> +      reg:
> >> +        description: |
> >> +          Power domain index. Valid values are defined in:
> >> +              "include/dt-bindings/power/mt8173-power.h" - for MT8173 type power domain.
> >> +        maxItems: 1
> >> +
> >> +      '#power-domain-cells':
> >> +        description:
> >> +          Documented by the generic PM Domain bindings in
> >> +          Documentation/devicetree/bindings/power/power-domain.yaml.
> >
> > No need to redefine a common property. This should define valid values
> > for it.
> >
> >> +
> >> +      clocks:
> >> +        description: |
> >> +          A number of phandles to clocks that need to be enabled during domain
> >> +          power-up sequencing.
> >
> > No need to redefine 'clocks'. You need to define how many, what each one
> > is, and the order.
> >
>
> Do you mean we have to define each clock for each power domain of each SoC?

Yes.

Rob

Re: [PATCH 01/12] dt-bindings: power: Add bindings for the Mediatek SCPSYS power domains controller

[Enric Balletbo i Serra]

Hi Rob,

Thank you for your answer. I have some doubts, see below.


On 23/9/20 0:36, Rob Herring wrote:
> On Mon, Sep 14, 2020 at 2:59 AM Matthias Brugger <matthias.bgg@gmail.com> wrote:
>>
>>
>>
>> On 12/09/2020 01:02, Rob Herring wrote:
>>> On Thu, Sep 10, 2020 at 07:28:15PM +0200, Enric Balletbo i Serra wrote:
>>>> The System Control Processor System (SCPSYS) has several power management
>>>> related tasks in the system. Add the bindings to define the power
>>>> domains for the SCPSYS power controller.
>>>>
>>>> Co-developed-by: Matthias Brugger <mbrugger@suse.com>
>>>> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
>>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>>>> ---
>>>> Dear Rob,
>>>>
>>>> I am awasre that this binding is not ready, but I prefered to send because I'm
>>>> kind of blocked. Compiling this binding triggers the following error:
>>>>
>>>>      mediatek,power-controller.example.dt.yaml: syscon@10006000: mfg_async@7:
>>>>      '#address-cells', '#size-cells', 'mfg_2d@8'
>>>>      do not match any of the regexes: 'pinctrl-[0-9]+'
>>>>
>>>> This happens when a definition of a power-domain (parent) contains
>>>> another power-domain (child), like the example. I am not sure how to
>>>> specify this in the yaml and deal with this, so any clue is welcome.
>>>
>>> You just have to keep nesting schemas all the way down. Define a
>>> grandchild node under the child node and then all of its properties.
>>>
>>>>
>>>> Thanks,
>>>>    Enric
>>>>
>>>>   .../power/mediatek,power-controller.yaml      | 171 ++++++++++++++++++
>>>>   1 file changed, 171 insertions(+)
>>>>   create mode 100644 Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
>>>> new file mode 100644
>>>> index 000000000000..8be9244ad160
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
>>>> @@ -0,0 +1,171 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/power/mediatek,power-controller.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Mediatek Power Domains Controller
>>>> +
>>>> +maintainers:
>>>> +  - Weiyi Lu <weiyi.lu@mediatek.com>
>>>> +  - Matthias Brugger <mbrugger@suse.com>
>>>> +
>>>> +description: |
>>>> +  Mediatek processors include support for multiple power domains which can be
>>>> +  powered up/down by software based on different application scenes to save power.
>>>> +
>>>> +  IP cores belonging to a power domain should contain a 'power-domains'
>>>> +  property that is a phandle for SCPSYS node representing the domain.
>>>> +
>>>> +properties:
>>>> +  $nodename:
>>>> +    pattern: "^syscon@[0-9a-f]+$"
>>>> +
>>>> +  compatible:
>>>> +    items:
>>>> +      - enum:
>>>> +        - mediatek,mt8173-power-controller
>>>> +      - const: syscon
>>>> +
>>>> +  reg:
>>>> +    maxItems: 1
>>>> +
>>>> +patternProperties:
>>>> +  "^.*@[0-9]$":
>>>
>>> Node names should be generic:
>>>
>>> power-domain@
>>>
>>
>> Enric correct me if I'm wrong, if we want to see the power domains in debugfs,
>> they are listed by their name. If all are called power-domain then the listing
>> is pretty much useless.
> 
> Sorry, but not a binding problem.
> 
> Maybe if debugfs shows what devices are contained within a power
> domain then it doesn't matter so much.
> 
>>>> +    type: object
>>>> +    description: |
>>>> +      Represents the power domains within the power controller node as documented
>>>> +      in Documentation/devicetree/bindings/power/power-domain.yaml.
>>>> +
>>>> +    properties:
>>>> +      reg:
>>>> +        description: |
>>>> +          Power domain index. Valid values are defined in:
>>>> +              "include/dt-bindings/power/mt8173-power.h" - for MT8173 type power domain.
>>>> +        maxItems: 1
>>>> +
>>>> +      '#power-domain-cells':
>>>> +        description:
>>>> +          Documented by the generic PM Domain bindings in
>>>> +          Documentation/devicetree/bindings/power/power-domain.yaml.
>>>
>>> No need to redefine a common property. This should define valid values
>>> for it.
>>>
>>>> +
>>>> +      clocks:
>>>> +        description: |
>>>> +          A number of phandles to clocks that need to be enabled during domain
>>>> +          power-up sequencing.
>>>
>>> No need to redefine 'clocks'. You need to define how many, what each one
>>> is, and the order.
>>>
>>
>> Do you mean we have to define each clock for each power domain of each SoC?
> 
> Yes.
> 

But every power domain can use totally different clocks I.e in the scpsys
controller for MT8173 we can have:

clocks:
    items:
      - description: MMSYS reference clock
      - description: Video Encoder reference clock

That matches with:

+            power-domain@MT8173_POWER_DOMAIN_VENC {
+                reg = <MT8173_POWER_DOMAIN_VENC>;
+                clocks = <&topckgen CLK_TOP_MM_SEL>,
+                         <&topckgen CLK_TOP_VENC_SEL>;
+                clock-names = "mm", "venc";
+                #power-domain-cells = <0>;
+            };

But then we can have another power-domain (inside) with another clock.

+            power-domain@MT8173_POWER_DOMAIN_MFG_ASYNC {
+                reg = <MT8173_POWER_DOMAIN_MFG_ASYNC>;
+                clocks = <&clk26m>;
+                clock-names = "mfg";
+            };

Should we add a new item like this then?

      - description: 26MHz reference clock

But clocks will not match with the order for this power-domain (as MMSYS and
VENC clocks are missing here). AFAIK is not possible to specify different clocks
for different power-domains that are inside the SCPSYS power controller node.

For other SoCs we can have something more complex like this:

+	power-domain@MT8183_POWER_DOMAIN_CAM {
+		reg = <MT8183_POWER_DOMAIN_CAM>;
+		clocks = <&topckgen CLK_TOP_MUX_CAM>,
+			 <&camsys CLK_CAM_LARB6>,
+			 <&camsys CLK_CAM_LARB3>,
+			 <&camsys CLK_CAM_SENINF>,
+			 <&camsys CLK_CAM_CAMSV0>,
+			 <&camsys CLK_CAM_CAMSV1>,
+			 <&camsys CLK_CAM_CAMSV2>,
+			 <&camsys CLK_CAM_CCU>;
+		clock-names = "cam", "cam-0", "cam-1",
+			      "cam-2", "cam-3", "cam-4",
+			      "cam-5", "cam-6";
+		mediatek,infracfg = <&infracfg>;
+		mediatek,smi = <&smi_common>;
+		#power-domain-cells = <0>;
+	};

Thanks,
  Enric


> Rob
> 

Re: [PATCH 00/12] soc: mediatek: pm-domains: Add new driver for SCPSYS power domains controller

[Weiyi Lu]

On Thu, 2020-09-10 at 19:28 +0200, Enric Balletbo i Serra wrote:
> Dear all,
> 
> This is a new driver with the aim to deprecate the mtk-scpsys driver.
> The problem with that driver is that, in order to support more Mediatek
> SoCs you need to add some logic to handle properly the power-up
> sequence of newer Mediatek SoCs, doesn't handle parent-child power
> domains and need to hardcode all the clocks in the driver itself. The
> result is that the driver is getting bigger and bigger every time a
> new SoC needs to be supported.
> 

Hi Enric and Matthias,

First of all, thank you for the patch. But I'm worried the problem you
mentioned won't be solved even if we work on this new driver in the
future. My work on the MT8183 scpsys(now v17) is to implement the new
hardware logic. Here, I also see related patches, which means that these
new logics are necessary. Why can't we work on the original driver?
Meanwhile, I thought maybe we should separate the driver into general
control and platform data for each SoC, otherwise it'll keep getting
bigger and bigger if it need to be support new SoC.

And consider DVFSRC
(dynamic voltage and frequency scaling resource collector), should we
keep the original driver name "scpsys" instead of "pm-domains" because
it may provide more functions than power domains?

> All this information can be getted from a properly defined binding, so
> can be cleaner and smaller, hence, we implemented a new driver. For
> now, only MT8173 and MT8183 is supported but should be fairly easy to
> add support for new SoCs.
> 
> Best regards,
>   Enric
> 
> Enric Balletbo i Serra (4):
>   dt-bindings: power: Add bindings for the Mediatek SCPSYS power domains
>     controller
>   soc: mediatek: Add MediaTek SCPSYS power domains
>   arm64: dts: mediatek: Add mt8173 power domain controller
>   dt-bindings: power: Add MT8183 power domains
> 
> Matthias Brugger (8):
>   soc: mediatek: pm-domains: Add bus protection protocol
>   soc: mediatek: pm_domains: Make bus protection generic
>   soc: mediatek: pm-domains: Add SMI block as bus protection block
>   soc: mediatek: pm-domains: Add extra sram control
>   soc: mediatek: pm-domains: Add subsystem clocks
>   soc: mediatek: pm-domains: Allow bus protection to ignore clear ack
>   soc: mediatek: pm-domains: Add support for mt8183
>   arm64: dts: mediatek: Add mt8183 power domains controller
> 
>  .../power/mediatek,power-controller.yaml      | 173 ++++
>  arch/arm64/boot/dts/mediatek/mt8173.dtsi      |  78 +-
>  arch/arm64/boot/dts/mediatek/mt8183.dtsi      | 160 +++
>  drivers/soc/mediatek/Kconfig                  |  13 +
>  drivers/soc/mediatek/Makefile                 |   1 +
>  drivers/soc/mediatek/mtk-infracfg.c           |   5 -
>  drivers/soc/mediatek/mtk-pm-domains.c         | 952 ++++++++++++++++++
>  include/dt-bindings/power/mt8183-power.h      |  26 +
>  include/linux/soc/mediatek/infracfg.h         |  39 +
>  9 files changed, 1433 insertions(+), 14 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
>  create mode 100644 drivers/soc/mediatek/mtk-pm-domains.c
>  create mode 100644 include/dt-bindings/power/mt8183-power.h
> 

Re: [PATCH 00/12] soc: mediatek: pm-domains: Add new driver for SCPSYS power domains controller

[Matthias Brugger]

On 25/09/2020 12:06, Weiyi Lu wrote:
> On Thu, 2020-09-10 at 19:28 +0200, Enric Balletbo i Serra wrote:
>> Dear all,
>>
>> This is a new driver with the aim to deprecate the mtk-scpsys driver.
>> The problem with that driver is that, in order to support more Mediatek
>> SoCs you need to add some logic to handle properly the power-up
>> sequence of newer Mediatek SoCs, doesn't handle parent-child power
>> domains and need to hardcode all the clocks in the driver itself. The
>> result is that the driver is getting bigger and bigger every time a
>> new SoC needs to be supported.
>>
> 
> Hi Enric and Matthias,
> 
> First of all, thank you for the patch. But I'm worried the problem you
> mentioned won't be solved even if we work on this new driver in the
> future. My work on the MT8183 scpsys(now v17) is to implement the new
> hardware logic. Here, I also see related patches, which means that these
> new logics are necessary. Why can't we work on the original driver?

Well the decision was to change the driver in a not compatible way to make 
device tree entries better. If we work on the old driver, we would need to find 
some creative ways to handle old bindings vs new bindings.

So I thought it would be better doing a fresh start implementing mt1873 support 
for reference and add mt8183 as new SoC. From what I have seen mt8192 and others 
fit the driver structure too.

> Meanwhile, I thought maybe we should separate the driver into general
> control and platform data for each SoC, otherwise it'll keep getting
> bigger and bigger if it need to be support new SoC.
> 

We could in a later series split the SoC depended data structures and put them 
in drivers/soc/mediatek/pm-domains-mt8183.h or something like this. Is that what 
you mean?

> And consider DVFSRC
> (dynamic voltage and frequency scaling resource collector), should we
> keep the original driver name "scpsys" instead of "pm-domains" because
> it may provide more functions than power domains?
> 

It's on my list to look deeper into this series. The thing with the new driver 
is, that the binding takes into account, that scpsys has several hardware block, 
which are represented as child nodes in DTS. The pm-domains is just one of these 
functionalities and I think DVFSRC should be a new driver with a child node of 
scpsys in DTS. Does this make sense?

Regards,
Matthias

>> All this information can be getted from a properly defined binding, so
>> can be cleaner and smaller, hence, we implemented a new driver. For
>> now, only MT8173 and MT8183 is supported but should be fairly easy to
>> add support for new SoCs.
>>
>> Best regards,
>>    Enric
>>
>> Enric Balletbo i Serra (4):
>>    dt-bindings: power: Add bindings for the Mediatek SCPSYS power domains
>>      controller
>>    soc: mediatek: Add MediaTek SCPSYS power domains
>>    arm64: dts: mediatek: Add mt8173 power domain controller
>>    dt-bindings: power: Add MT8183 power domains
>>
>> Matthias Brugger (8):
>>    soc: mediatek: pm-domains: Add bus protection protocol
>>    soc: mediatek: pm_domains: Make bus protection generic
>>    soc: mediatek: pm-domains: Add SMI block as bus protection block
>>    soc: mediatek: pm-domains: Add extra sram control
>>    soc: mediatek: pm-domains: Add subsystem clocks
>>    soc: mediatek: pm-domains: Allow bus protection to ignore clear ack
>>    soc: mediatek: pm-domains: Add support for mt8183
>>    arm64: dts: mediatek: Add mt8183 power domains controller
>>
>>   .../power/mediatek,power-controller.yaml      | 173 ++++
>>   arch/arm64/boot/dts/mediatek/mt8173.dtsi      |  78 +-
>>   arch/arm64/boot/dts/mediatek/mt8183.dtsi      | 160 +++
>>   drivers/soc/mediatek/Kconfig                  |  13 +
>>   drivers/soc/mediatek/Makefile                 |   1 +
>>   drivers/soc/mediatek/mtk-infracfg.c           |   5 -
>>   drivers/soc/mediatek/mtk-pm-domains.c         | 952 ++++++++++++++++++
>>   include/dt-bindings/power/mt8183-power.h      |  26 +
>>   include/linux/soc/mediatek/infracfg.h         |  39 +
>>   9 files changed, 1433 insertions(+), 14 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
>>   create mode 100644 drivers/soc/mediatek/mtk-pm-domains.c
>>   create mode 100644 include/dt-bindings/power/mt8183-power.h
>>
> 

Re: [PATCH 00/12] soc: mediatek: pm-domains: Add new driver for SCPSYS power domains controller

[Weiyi Lu]

On Fri, 2020-09-25 at 16:04 +0200, Matthias Brugger wrote:
> 
> On 25/09/2020 12:06, Weiyi Lu wrote:
> > On Thu, 2020-09-10 at 19:28 +0200, Enric Balletbo i Serra wrote:
> >> Dear all,
> >>
> >> This is a new driver with the aim to deprecate the mtk-scpsys driver.
> >> The problem with that driver is that, in order to support more Mediatek
> >> SoCs you need to add some logic to handle properly the power-up
> >> sequence of newer Mediatek SoCs, doesn't handle parent-child power
> >> domains and need to hardcode all the clocks in the driver itself. The
> >> result is that the driver is getting bigger and bigger every time a
> >> new SoC needs to be supported.
> >>
> > 
> > Hi Enric and Matthias,
> > 
> > First of all, thank you for the patch. But I'm worried the problem you
> > mentioned won't be solved even if we work on this new driver in the
> > future. My work on the MT8183 scpsys(now v17) is to implement the new
> > hardware logic. Here, I also see related patches, which means that these
> > new logics are necessary. Why can't we work on the original driver?
> 
> Well the decision was to change the driver in a not compatible way to make 
> device tree entries better. If we work on the old driver, we would need to find 
> some creative ways to handle old bindings vs new bindings.
> 
> So I thought it would be better doing a fresh start implementing mt1873 support 
> for reference and add mt8183 as new SoC. From what I have seen mt8192 and others 
> fit the driver structure too.
> 
> > Meanwhile, I thought maybe we should separate the driver into general
> > control and platform data for each SoC, otherwise it'll keep getting
> > bigger and bigger if it need to be support new SoC.
> > 
> 
> We could in a later series split the SoC depended data structures and put them 
> in drivers/soc/mediatek/pm-domains-mt8183.h or something like this. Is that what 
> you mean?
> 

Yes, that is what I want. And I guess it could avoid the collisions in
the different defines to the control registers and power status bits you
mentioned. Hope this will happen in this series.

> > And consider DVFSRC
> > (dynamic voltage and frequency scaling resource collector), should we
> > keep the original driver name "scpsys" instead of "pm-domains" because
> > it may provide more functions than power domains?
> > 
> 
> It's on my list to look deeper into this series. The thing with the new driver 
> is, that the binding takes into account, that scpsys has several hardware block, 
> which are represented as child nodes in DTS. The pm-domains is just one of these 
> functionalities and I think DVFSRC should be a new driver with a child node of 
> scpsys in DTS. Does this make sense?
> 
> Regards,
> Matthias
> 
> >> All this information can be getted from a properly defined binding, so
> >> can be cleaner and smaller, hence, we implemented a new driver. For
> >> now, only MT8173 and MT8183 is supported but should be fairly easy to
> >> add support for new SoCs.
> >>
> >> Best regards,
> >>    Enric
> >>
> >> Enric Balletbo i Serra (4):
> >>    dt-bindings: power: Add bindings for the Mediatek SCPSYS power domains
> >>      controller
> >>    soc: mediatek: Add MediaTek SCPSYS power domains
> >>    arm64: dts: mediatek: Add mt8173 power domain controller
> >>    dt-bindings: power: Add MT8183 power domains
> >>
> >> Matthias Brugger (8):
> >>    soc: mediatek: pm-domains: Add bus protection protocol
> >>    soc: mediatek: pm_domains: Make bus protection generic
> >>    soc: mediatek: pm-domains: Add SMI block as bus protection block
> >>    soc: mediatek: pm-domains: Add extra sram control
> >>    soc: mediatek: pm-domains: Add subsystem clocks
> >>    soc: mediatek: pm-domains: Allow bus protection to ignore clear ack
> >>    soc: mediatek: pm-domains: Add support for mt8183
> >>    arm64: dts: mediatek: Add mt8183 power domains controller
> >>
> >>   .../power/mediatek,power-controller.yaml      | 173 ++++
> >>   arch/arm64/boot/dts/mediatek/mt8173.dtsi      |  78 +-
> >>   arch/arm64/boot/dts/mediatek/mt8183.dtsi      | 160 +++
> >>   drivers/soc/mediatek/Kconfig                  |  13 +
> >>   drivers/soc/mediatek/Makefile                 |   1 +
> >>   drivers/soc/mediatek/mtk-infracfg.c           |   5 -
> >>   drivers/soc/mediatek/mtk-pm-domains.c         | 952 ++++++++++++++++++
> >>   include/dt-bindings/power/mt8183-power.h      |  26 +
> >>   include/linux/soc/mediatek/infracfg.h         |  39 +
> >>   9 files changed, 1433 insertions(+), 14 deletions(-)
> >>   create mode 100644 Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> >>   create mode 100644 drivers/soc/mediatek/mtk-pm-domains.c
> >>   create mode 100644 include/dt-bindings/power/mt8183-power.h
> >>
> > 
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 00/12] soc: mediatek: pm-domains: Add new driver for SCPSYS power domains controller

[Matthias Brugger]

On 06/10/2020 08:53, Weiyi Lu wrote:
> On Fri, 2020-09-25 at 16:04 +0200, Matthias Brugger wrote:
>>
>> On 25/09/2020 12:06, Weiyi Lu wrote:
>>> On Thu, 2020-09-10 at 19:28 +0200, Enric Balletbo i Serra wrote:
>>>> Dear all,
>>>>
>>>> This is a new driver with the aim to deprecate the mtk-scpsys driver.
>>>> The problem with that driver is that, in order to support more Mediatek
>>>> SoCs you need to add some logic to handle properly the power-up
>>>> sequence of newer Mediatek SoCs, doesn't handle parent-child power
>>>> domains and need to hardcode all the clocks in the driver itself. The
>>>> result is that the driver is getting bigger and bigger every time a
>>>> new SoC needs to be supported.
>>>>
>>>
>>> Hi Enric and Matthias,
>>>
>>> First of all, thank you for the patch. But I'm worried the problem you
>>> mentioned won't be solved even if we work on this new driver in the
>>> future. My work on the MT8183 scpsys(now v17) is to implement the new
>>> hardware logic. Here, I also see related patches, which means that these
>>> new logics are necessary. Why can't we work on the original driver?
>>
>> Well the decision was to change the driver in a not compatible way to make
>> device tree entries better. If we work on the old driver, we would need to find
>> some creative ways to handle old bindings vs new bindings.
>>
>> So I thought it would be better doing a fresh start implementing mt1873 support
>> for reference and add mt8183 as new SoC. From what I have seen mt8192 and others
>> fit the driver structure too.
>>
>>> Meanwhile, I thought maybe we should separate the driver into general
>>> control and platform data for each SoC, otherwise it'll keep getting
>>> bigger and bigger if it need to be support new SoC.
>>>
>>
>> We could in a later series split the SoC depended data structures and put them
>> in drivers/soc/mediatek/pm-domains-mt8183.h or something like this. Is that what
>> you mean?
>>
> 
> Yes, that is what I want. And I guess it could avoid the collisions in
> the different defines to the control registers and power status bits you
> mentioned. Hope this will happen in this series.
> 

Sounds good to me. Enric could you move the soc specific data to separate 
include files?

Regards,
Matthias

Re: [PATCH 00/12] soc: mediatek: pm-domains: Add new driver for SCPSYS power domains controller

[Enric Balletbo i Serra]

Hi,

On 9/10/20 14:50, Matthias Brugger wrote:
> 
> 
> On 06/10/2020 08:53, Weiyi Lu wrote:
>> On Fri, 2020-09-25 at 16:04 +0200, Matthias Brugger wrote:
>>>
>>> On 25/09/2020 12:06, Weiyi Lu wrote:
>>>> On Thu, 2020-09-10 at 19:28 +0200, Enric Balletbo i Serra wrote:
>>>>> Dear all,
>>>>>
>>>>> This is a new driver with the aim to deprecate the mtk-scpsys driver.
>>>>> The problem with that driver is that, in order to support more Mediatek
>>>>> SoCs you need to add some logic to handle properly the power-up
>>>>> sequence of newer Mediatek SoCs, doesn't handle parent-child power
>>>>> domains and need to hardcode all the clocks in the driver itself. The
>>>>> result is that the driver is getting bigger and bigger every time a
>>>>> new SoC needs to be supported.
>>>>>
>>>>
>>>> Hi Enric and Matthias,
>>>>
>>>> First of all, thank you for the patch. But I'm worried the problem you
>>>> mentioned won't be solved even if we work on this new driver in the
>>>> future. My work on the MT8183 scpsys(now v17) is to implement the new
>>>> hardware logic. Here, I also see related patches, which means that these
>>>> new logics are necessary. Why can't we work on the original driver?
>>>
>>> Well the decision was to change the driver in a not compatible way to make
>>> device tree entries better. If we work on the old driver, we would need to find
>>> some creative ways to handle old bindings vs new bindings.
>>>
>>> So I thought it would be better doing a fresh start implementing mt1873 support
>>> for reference and add mt8183 as new SoC. From what I have seen mt8192 and others
>>> fit the driver structure too.
>>>
>>>> Meanwhile, I thought maybe we should separate the driver into general
>>>> control and platform data for each SoC, otherwise it'll keep getting
>>>> bigger and bigger if it need to be support new SoC.
>>>>
>>>
>>> We could in a later series split the SoC depended data structures and put them
>>> in drivers/soc/mediatek/pm-domains-mt8183.h or something like this. Is that what
>>> you mean?
>>>
>>
>> Yes, that is what I want. And I guess it could avoid the collisions in
>> the different defines to the control registers and power status bits you
>> mentioned. Hope this will happen in this series.
>>
> 
> Sounds good to me. Enric could you move the soc specific data to separate
> include files?
> 

Sure, I'll do this in v4.

Thanks,
 Enric

> Regards,
> Matthias
>