[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 02/12] soc: mediatek: Add MediaTek SCPSYS power domains

[Enric Balletbo i Serra]

The System Control Processor System (SCPSYS) has several power management
related tasks in the system. This driver implements support to handle
the different power domains supported in order to meet high performance
and low power requirements.

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

 drivers/soc/mediatek/Kconfig          |  13 +
 drivers/soc/mediatek/Makefile         |   1 +
 drivers/soc/mediatek/mtk-pm-domains.c | 626 ++++++++++++++++++++++++++
 3 files changed, 640 insertions(+)
 create mode 100644 drivers/soc/mediatek/mtk-pm-domains.c

diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
index 59a56cd790ec..68d800f9e4a5 100644
--- a/drivers/soc/mediatek/Kconfig
+++ b/drivers/soc/mediatek/Kconfig
@@ -44,6 +44,19 @@ config MTK_SCPSYS
 	  Say yes here to add support for the MediaTek SCPSYS power domain
 	  driver.
 
+config MTK_SCPSYS_PM_DOMAINS
+	bool "MediaTek SCPSYS generic power domain"
+	default ARCH_MEDIATEK
+	depends on PM
+	depends on MTK_INFRACFG
+	select PM_GENERIC_DOMAINS
+	select REGMAP
+	help
+	  Say y here to enable power domain support.
+	  In order to meet high performance and low power requirements, the System
+	  Control Processor System (SCPSYS) has several power management related
+	  tasks in the system.
+
 config MTK_MMSYS
 	bool "MediaTek MMSYS Support"
 	default ARCH_MEDIATEK
diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
index 01f9f873634a..1e60fb4f89d4 100644
--- a/drivers/soc/mediatek/Makefile
+++ b/drivers/soc/mediatek/Makefile
@@ -3,4 +3,5 @@ obj-$(CONFIG_MTK_CMDQ) += mtk-cmdq-helper.o
 obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o
 obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o
 obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o
+obj-$(CONFIG_MTK_SCPSYS_PM_DOMAINS) += mtk-pm-domains.o
 obj-$(CONFIG_MTK_MMSYS) += mtk-mmsys.o
diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
new file mode 100644
index 000000000000..db631dbaf2e3
--- /dev/null
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -0,0 +1,626 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2020 Collabora Ltd.
+ */
+#include <linux/clk.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of_clk.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/soc/mediatek/infracfg.h>
+
+#include <dt-bindings/power/mt8173-power.h>
+
+#define MTK_POLL_DELAY_US   10
+#define MTK_POLL_TIMEOUT    USEC_PER_SEC
+
+#define MTK_SCPD_ACTIVE_WAKEUP		BIT(0)
+#define MTK_SCPD_FWAIT_SRAM		BIT(1)
+#define MTK_SCPD_CAPS(_scpd, _x)	((_scpd)->data->caps & (_x))
+
+#define SPM_VDE_PWR_CON			0x0210
+#define SPM_MFG_PWR_CON			0x0214
+#define SPM_VEN_PWR_CON			0x0230
+#define SPM_ISP_PWR_CON			0x0238
+#define SPM_DIS_PWR_CON			0x023c
+#define SPM_VEN2_PWR_CON		0x0298
+#define SPM_AUDIO_PWR_CON		0x029c
+#define SPM_MFG_2D_PWR_CON		0x02c0
+#define SPM_MFG_ASYNC_PWR_CON		0x02c4
+#define SPM_USB_PWR_CON			0x02cc
+
+#define SPM_PWR_STATUS			0x060c
+#define SPM_PWR_STATUS_2ND		0x0610
+
+#define PWR_RST_B_BIT			BIT(0)
+#define PWR_ISO_BIT			BIT(1)
+#define PWR_ON_BIT			BIT(2)
+#define PWR_ON_2ND_BIT			BIT(3)
+#define PWR_CLK_DIS_BIT			BIT(4)
+
+#define PWR_STATUS_DISP			BIT(3)
+#define PWR_STATUS_MFG			BIT(4)
+#define PWR_STATUS_ISP			BIT(5)
+#define PWR_STATUS_VDEC			BIT(7)
+#define PWR_STATUS_VENC_LT		BIT(20)
+#define PWR_STATUS_VENC			BIT(21)
+#define PWR_STATUS_MFG_2D		BIT(22)
+#define PWR_STATUS_MFG_ASYNC		BIT(23)
+#define PWR_STATUS_AUDIO		BIT(24)
+#define PWR_STATUS_USB			BIT(25)
+
+struct scpsys_bus_prot_data {
+	u32 bus_prot_mask;
+	bool bus_prot_reg_update;
+};
+
+/**
+ * struct scpsys_domain_data - scp domain data for power on/off flow
+ * @sta_mask: The mask for power on/off status bit.
+ * @ctl_offs: The offset for main power control register.
+ * @sram_pdn_bits: The mask for sram power control bits.
+ * @sram_pdn_ack_bits: The mask for sram power control acked bits.
+ * @caps: The flag for active wake-up action.
+ * @bp_infracfg: bus protection for infracfg subsystem
+ */
+struct scpsys_domain_data {
+	u32 sta_mask;
+	int ctl_offs;
+	u32 sram_pdn_bits;
+	u32 sram_pdn_ack_bits;
+	u8 caps;
+	const struct scpsys_bus_prot_data bp_infracfg;
+};
+
+struct scpsys_domain {
+	struct generic_pm_domain genpd;
+	const struct scpsys_domain_data *data;
+	struct scpsys *scpsys;
+	int num_clks;
+	struct clk_bulk_data *clks;
+	struct regmap *infracfg;
+};
+
+struct scpsys_soc_data {
+	const struct scpsys_domain_data *domains;
+	int num_domains;
+	int pwr_sta_offs;
+	int pwr_sta2nd_offs;
+};
+
+struct scpsys {
+	struct device *dev;
+	void __iomem *base;
+	const struct scpsys_soc_data *soc_data;
+	struct genpd_onecell_data pd_data;
+	struct generic_pm_domain *domains[];
+};
+
+#define to_scpsys_domain(gpd) container_of(gpd, struct scpsys_domain, genpd)
+
+static int scpsys_domain_is_on(struct scpsys_domain *pd)
+{
+	struct scpsys *scpsys = pd->scpsys;
+
+	u32 status = readl(scpsys->base + scpsys->soc_data->pwr_sta_offs) & pd->data->sta_mask;
+	u32 status2 = readl(scpsys->base + scpsys->soc_data->pwr_sta2nd_offs) & pd->data->sta_mask;
+
+	/*
+	 * A domain is on when both status bits are set. If only one is set
+	 * return an error. This happens while powering up a domain
+	 */
+
+	if (status && status2)
+		return true;
+	if (!status && !status2)
+		return false;
+
+	return -EINVAL;
+}
+
+static int scpsys_sram_enable(struct scpsys_domain *pd, void __iomem *ctl_addr)
+{
+	u32 pdn_ack = pd->data->sram_pdn_ack_bits;
+	u32 val;
+	int tmp;
+	int ret;
+
+	val = readl(ctl_addr);
+	val &= ~pd->data->sram_pdn_bits;
+	writel(val, ctl_addr);
+
+	/* Either wait until SRAM_PDN_ACK all 1 or 0 */
+	ret = readl_poll_timeout(ctl_addr, tmp, (tmp & pdn_ack) == 0, MTK_POLL_DELAY_US,
+				 MTK_POLL_TIMEOUT);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int scpsys_sram_disable(struct scpsys_domain *pd, void __iomem *ctl_addr)
+{
+	u32 pdn_ack = pd->data->sram_pdn_ack_bits;
+	u32 val;
+	int tmp;
+
+	val = readl(ctl_addr);
+	val |= pd->data->sram_pdn_bits;
+	writel(val, ctl_addr);
+
+	/* Either wait until SRAM_PDN_ACK all 1 or 0 */
+	return readl_poll_timeout(ctl_addr, tmp, (tmp & pdn_ack) == pdn_ack, MTK_POLL_DELAY_US,
+				  MTK_POLL_TIMEOUT);
+}
+
+static int scpsys_bus_protect_enable(struct scpsys_domain *pd)
+{
+	const struct scpsys_bus_prot_data *bp_data = &pd->data->bp_infracfg;
+
+	if (!bp_data->bus_prot_mask)
+		return 0;
+
+	return mtk_infracfg_set_bus_protection(pd->infracfg, bp_data->bus_prot_mask,
+					       bp_data->bus_prot_reg_update);
+}
+
+static int scpsys_bus_protect_disable(struct scpsys_domain *pd)
+{
+	const struct scpsys_bus_prot_data *bp_data = &pd->data->bp_infracfg;
+
+	if (!bp_data->bus_prot_mask)
+		return 0;
+
+	return mtk_infracfg_clear_bus_protection(pd->infracfg, bp_data->bus_prot_mask,
+						 bp_data->bus_prot_reg_update);
+}
+
+static int scpsys_power_on(struct generic_pm_domain *genpd)
+{
+	struct scpsys_domain *pd = container_of(genpd, struct scpsys_domain, genpd);
+	struct scpsys *scpsys = pd->scpsys;
+	void __iomem *ctl_addr = scpsys->base + pd->data->ctl_offs;
+	int ret, tmp;
+	u32 val;
+
+	ret = clk_bulk_enable(pd->num_clks, pd->clks);
+	if (ret)
+		return ret;
+
+	/* subsys power on */
+	val = readl(ctl_addr);
+	val |= PWR_ON_BIT;
+	writel(val, ctl_addr);
+	val |= PWR_ON_2ND_BIT;
+	writel(val, ctl_addr);
+
+	/* wait until PWR_ACK = 1 */
+	ret = readx_poll_timeout(scpsys_domain_is_on, pd, tmp, tmp > 0, MTK_POLL_DELAY_US,
+				 MTK_POLL_TIMEOUT);
+	if (ret < 0)
+		goto err_pwr_ack;
+
+	val &= ~PWR_CLK_DIS_BIT;
+	writel(val, ctl_addr);
+
+	val &= ~PWR_ISO_BIT;
+	writel(val, ctl_addr);
+
+	val |= PWR_RST_B_BIT;
+	writel(val, ctl_addr);
+
+	ret = scpsys_sram_enable(pd, ctl_addr);
+	if (ret < 0)
+		goto err_pwr_ack;
+
+	ret = scpsys_bus_protect_disable(pd);
+	if (ret < 0)
+		goto err_pwr_ack;
+
+	return 0;
+
+err_pwr_ack:
+	clk_bulk_disable(pd->num_clks, pd->clks);
+	dev_err(scpsys->dev, "Failed to power on domain %s\n", genpd->name);
+
+	return ret;
+}
+
+static int scpsys_power_off(struct generic_pm_domain *genpd)
+{
+	struct scpsys_domain *pd = container_of(genpd, struct scpsys_domain, genpd);
+	struct scpsys *scpsys = pd->scpsys;
+	void __iomem *ctl_addr = scpsys->base + pd->data->ctl_offs;
+	int ret, tmp;
+	u32 val;
+
+	ret = scpsys_bus_protect_enable(pd);
+	if (ret < 0)
+		return ret;
+
+	ret = scpsys_sram_disable(pd, ctl_addr);
+	if (ret < 0)
+		return ret;
+
+	/* subsys power off */
+	val = readl(ctl_addr);
+	val |= PWR_ISO_BIT;
+	writel(val, ctl_addr);
+
+	val &= ~PWR_RST_B_BIT;
+	writel(val, ctl_addr);
+
+	val |= PWR_CLK_DIS_BIT;
+	writel(val, ctl_addr);
+
+	val &= ~PWR_ON_BIT;
+	writel(val, ctl_addr);
+
+	val &= ~PWR_ON_2ND_BIT;
+	writel(val, ctl_addr);
+
+	/* wait until PWR_ACK = 0 */
+	ret = readx_poll_timeout(scpsys_domain_is_on, pd, tmp, tmp == 0, MTK_POLL_DELAY_US,
+				 MTK_POLL_TIMEOUT);
+	if (ret < 0)
+		return ret;
+
+	clk_bulk_disable(pd->num_clks, pd->clks);
+
+	return 0;
+}
+
+static int scpsys_add_one_domain(struct scpsys *scpsys, struct device_node *node)
+{
+	const struct scpsys_domain_data *domain_data;
+	struct scpsys_domain *pd;
+	int i, ret;
+	u32 id;
+
+	ret = of_property_read_u32(node, "reg", &id);
+	if (ret) {
+		dev_err(scpsys->dev, "%pOFn: failed to retrieve domain id from reg: %d\n", node,
+			ret);
+		return -EINVAL;
+	}
+
+	if (id >= scpsys->soc_data->num_domains) {
+		dev_err(scpsys->dev, "%pOFn: invalid domain id %d\n", node, id);
+		return -EINVAL;
+	}
+
+	domain_data = &scpsys->soc_data->domains[id];
+	if (!domain_data) {
+		dev_err(scpsys->dev, "%pOFn: undefined domain id %d\n", node, id);
+		return -EINVAL;
+	}
+
+	pd = devm_kzalloc(scpsys->dev, sizeof(*pd), GFP_KERNEL);
+	if (!pd)
+		return -ENOMEM;
+
+	pd->data = domain_data;
+	pd->scpsys = scpsys;
+
+	pd->infracfg = syscon_regmap_lookup_by_phandle(node, "mediatek,infracfg");
+	if (IS_ERR(pd->infracfg))
+		pd->infracfg = NULL;
+
+	pd->num_clks = of_clk_get_parent_count(node);
+	if (pd->num_clks > 0) {
+		pd->clks = devm_kcalloc(scpsys->dev, pd->num_clks, sizeof(*pd->clks), GFP_KERNEL);
+		if (!pd->clks)
+			return -ENOMEM;
+	} else {
+		pd->num_clks = 0;
+	}
+
+	for (i = 0; i < pd->num_clks; i++) {
+		pd->clks[i].clk = of_clk_get(node, i);
+		if (IS_ERR(pd->clks[i].clk)) {
+			ret = PTR_ERR(pd->clks[i].clk);
+			dev_err(scpsys->dev, "%pOFn: failed to get clk at index %d: %d\n", node, i,
+				ret);
+			return ret;
+		}
+	}
+
+	ret = clk_bulk_prepare(pd->num_clks, pd->clks);
+	if (ret)
+		goto err_put_clocks;
+
+	/*
+	 * Initially turn on all domains to make the domains usable
+	 * with !CONFIG_PM and to get the hardware in sync with the
+	 * software.  The unused domains will be switched off during
+	 * late_init time.
+	 */
+	ret = scpsys_power_on(&pd->genpd);
+	if (ret < 0) {
+		dev_err(scpsys->dev, "failed to power on domain %pOFN with error %d\n", node, ret);
+		goto err_unprepare_clocks;
+	}
+
+	pd->genpd.name = node->name;
+	pd->genpd.power_off = scpsys_power_off;
+	pd->genpd.power_on = scpsys_power_on;
+
+	pm_genpd_init(&pd->genpd, NULL, false);
+
+	scpsys->domains[id] = &pd->genpd;
+	return 0;
+
+err_unprepare_clocks:
+	clk_bulk_unprepare(pd->num_clks, pd->clks);
+err_put_clocks:
+	clk_bulk_put(pd->num_clks, pd->clks);
+	devm_kfree(scpsys->dev, pd->clks);
+	pd->num_clks = 0;
+	return ret;
+}
+
+static int scpsys_add_subdomain(struct scpsys *scpsys, struct device_node *parent)
+{
+	struct device_node *child;
+	struct generic_pm_domain *child_pd, *parent_pd;
+	int ret;
+
+	for_each_child_of_node(parent, child) {
+		u32 id;
+
+		ret = of_property_read_u32(parent, "reg", &id);
+		if (ret) {
+			dev_err(scpsys->dev, "%pOFn: failed to get parent domain id: %d\n", child,
+				ret);
+			goto err_put_node;
+		}
+		parent_pd = scpsys->pd_data.domains[id];
+
+		ret = scpsys_add_one_domain(scpsys, child);
+		if (ret) {
+			dev_err(scpsys->dev, "error adding power domain for %pOFn: %d\n", child,
+				ret);
+			goto err_put_node;
+		}
+
+		ret = of_property_read_u32(child, "reg", &id);
+		if (ret) {
+			dev_err(scpsys->dev, "%pOFn: failed to get child domain id: %d\n", child,
+				ret);
+			goto err_put_node;
+		}
+		child_pd = scpsys->pd_data.domains[id];
+
+		ret = pm_genpd_add_subdomain(parent_pd, child_pd);
+		if (ret) {
+			dev_err(scpsys->dev, "failed to add %s subdomain to parent %s\n",
+				child_pd->name, parent_pd->name);
+			goto err_put_node;
+		} else {
+			dev_dbg(scpsys->dev, "%s add subdomain: %s\n", parent_pd->name,
+				child_pd->name);
+		}
+
+		/* recursive call to add all subdomains */
+		ret = scpsys_add_subdomain(scpsys, child);
+		if (ret)
+			goto err_put_node;
+	}
+
+	return 0;
+
+err_put_node:
+	of_node_put(child);
+	return ret;
+}
+
+static void scpsys_remove_one_domain(struct scpsys_domain *pd)
+{
+	int ret;
+
+	/*
+	 * We're in the error cleanup already, so we only complain,
+	 * but won't emit another error on top of the original one.
+	 */
+	ret = pm_genpd_remove(&pd->genpd);
+	if (ret < 0)
+		dev_err(pd->scpsys->dev,
+			"failed to remove domain '%s' : %d - state may be inconsistent\n",
+			pd->genpd.name, ret);
+
+	scpsys_power_off(&pd->genpd);
+
+	clk_bulk_unprepare(pd->num_clks, pd->clks);
+	clk_bulk_put(pd->num_clks, pd->clks);
+	pd->num_clks = 0;
+}
+
+static void scpsys_domain_cleanup(struct scpsys *scpsys)
+{
+	struct generic_pm_domain *genpd;
+	struct scpsys_domain *pd;
+	int i;
+
+	for (i = scpsys->pd_data.num_domains - 1; i >= 0; i--) {
+		genpd = scpsys->pd_data.domains[i];
+		if (genpd) {
+			pd = to_scpsys_domain(genpd);
+			scpsys_remove_one_domain(pd);
+		}
+	}
+}
+
+/*
+ * MT8173 power domain support
+ */
+
+static const struct scpsys_domain_data scpsys_domain_data_mt8173[] = {
+	[MT8173_POWER_DOMAIN_VDEC] = {
+		.sta_mask = PWR_STATUS_VDEC,
+		.ctl_offs = SPM_VDE_PWR_CON,
+		.sram_pdn_bits = GENMASK(11, 8),
+		.sram_pdn_ack_bits = GENMASK(12, 12),
+	},
+	[MT8173_POWER_DOMAIN_VENC] = {
+		.sta_mask = PWR_STATUS_VENC,
+		.ctl_offs = SPM_VEN_PWR_CON,
+		.sram_pdn_bits = GENMASK(11, 8),
+		.sram_pdn_ack_bits = GENMASK(15, 12),
+	},
+	[MT8173_POWER_DOMAIN_ISP] = {
+		.sta_mask = PWR_STATUS_ISP,
+		.ctl_offs = SPM_ISP_PWR_CON,
+		.sram_pdn_bits = GENMASK(11, 8),
+		.sram_pdn_ack_bits = GENMASK(13, 12),
+	},
+	[MT8173_POWER_DOMAIN_MM] = {
+		.sta_mask = PWR_STATUS_DISP,
+		.ctl_offs = SPM_DIS_PWR_CON,
+		.sram_pdn_bits = GENMASK(11, 8),
+		.sram_pdn_ack_bits = GENMASK(12, 12),
+		.bp_infracfg = {
+			.bus_prot_reg_update = true,
+			.bus_prot_mask = MT8173_TOP_AXI_PROT_EN_MM_M0 |
+				MT8173_TOP_AXI_PROT_EN_MM_M1,
+		},
+	},
+	[MT8173_POWER_DOMAIN_VENC_LT] = {
+		.sta_mask = PWR_STATUS_VENC_LT,
+		.ctl_offs = SPM_VEN2_PWR_CON,
+		.sram_pdn_bits = GENMASK(11, 8),
+		.sram_pdn_ack_bits = GENMASK(15, 12),
+	},
+	[MT8173_POWER_DOMAIN_AUDIO] = {
+		.sta_mask = PWR_STATUS_AUDIO,
+		.ctl_offs = SPM_AUDIO_PWR_CON,
+		.sram_pdn_bits = GENMASK(11, 8),
+		.sram_pdn_ack_bits = GENMASK(15, 12),
+	},
+	[MT8173_POWER_DOMAIN_USB] = {
+		.sta_mask = PWR_STATUS_USB,
+		.ctl_offs = SPM_USB_PWR_CON,
+		.sram_pdn_bits = GENMASK(11, 8),
+		.sram_pdn_ack_bits = GENMASK(15, 12),
+		.caps = MTK_SCPD_ACTIVE_WAKEUP,
+	},
+	[MT8173_POWER_DOMAIN_MFG_ASYNC] = {
+		.sta_mask = PWR_STATUS_MFG_ASYNC,
+		.ctl_offs = SPM_MFG_ASYNC_PWR_CON,
+		.sram_pdn_bits = GENMASK(11, 8),
+		.sram_pdn_ack_bits = 0,
+	},
+	[MT8173_POWER_DOMAIN_MFG_2D] = {
+		.sta_mask = PWR_STATUS_MFG_2D,
+		.ctl_offs = SPM_MFG_2D_PWR_CON,
+		.sram_pdn_bits = GENMASK(11, 8),
+		.sram_pdn_ack_bits = GENMASK(13, 12),
+	},
+	[MT8173_POWER_DOMAIN_MFG] = {
+		.sta_mask = PWR_STATUS_MFG,
+		.ctl_offs = SPM_MFG_PWR_CON,
+		.sram_pdn_bits = GENMASK(13, 8),
+		.sram_pdn_ack_bits = GENMASK(21, 16),
+		.bp_infracfg = {
+			.bus_prot_reg_update = true,
+			.bus_prot_mask = MT8173_TOP_AXI_PROT_EN_MFG_S |
+				MT8173_TOP_AXI_PROT_EN_MFG_M0 |
+				MT8173_TOP_AXI_PROT_EN_MFG_M1 |
+				MT8173_TOP_AXI_PROT_EN_MFG_SNOOP_OUT,
+		},
+	},
+};
+
+static const struct scpsys_soc_data mt8173_scpsys_data = {
+	.domains = scpsys_domain_data_mt8173,
+	.num_domains = ARRAY_SIZE(scpsys_domain_data_mt8173),
+	.pwr_sta_offs = SPM_PWR_STATUS,
+	.pwr_sta2nd_offs = SPM_PWR_STATUS_2ND,
+};
+
+static const struct of_device_id scpsys_of_match[] = {
+	{
+		.compatible = "mediatek,mt8173-power-controller",
+		.data = &mt8173_scpsys_data,
+	},
+	{ }
+};
+
+static int scpsys_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	const struct scpsys_soc_data *soc;
+	struct device_node *node;
+	struct scpsys *scpsys;
+	struct resource *res;
+	int ret;
+
+	soc = of_device_get_match_data(&pdev->dev);
+	if (!soc) {
+		dev_err(&pdev->dev, "no power controller data\n");
+		return -EINVAL;
+	}
+
+	scpsys = devm_kzalloc(dev, struct_size(scpsys, domains, soc->num_domains), GFP_KERNEL);
+	if (!scpsys)
+		return -ENOMEM;
+
+	scpsys->dev = dev;
+	scpsys->soc_data = soc;
+
+	scpsys->pd_data.domains = scpsys->domains;
+	scpsys->pd_data.num_domains = soc->num_domains;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	scpsys->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(scpsys->base))
+		return -ENODEV;
+
+	ret = -ENODEV;
+	for_each_available_child_of_node(np, node) {
+		ret = scpsys_add_one_domain(scpsys, node);
+		if (ret) {
+			dev_err(dev, "failed to handle node %pOFN: %d\n", node, ret);
+			of_node_put(node);
+			goto err_cleanup_domains;
+		}
+
+		ret = scpsys_add_subdomain(scpsys, node);
+		if (ret) {
+			dev_err(dev, "failed to add subdomain node %pOFn: %d\n", node, ret);
+			of_node_put(node);
+			goto err_cleanup_domains;
+		}
+	}
+
+	if (ret) {
+		dev_dbg(dev, "no power domains present\n");
+		return ret;
+	}
+
+	ret = of_genpd_add_provider_onecell(np, &scpsys->pd_data);
+	if (ret) {
+		dev_err(dev, "failed to add provider: %d\n", ret);
+		goto err_cleanup_domains;
+	}
+
+	return 0;
+
+err_cleanup_domains:
+	scpsys_domain_cleanup(scpsys);
+	return ret;
+}
+
+static struct platform_driver scpsys_pm_domain_driver = {
+	.probe = scpsys_probe,
+	.driver = {
+		.name = "mtk-power-controller",
+		.suppress_bind_attrs = true,
+		.of_match_table = scpsys_of_match,
+	},
+};
+builtin_platform_driver(scpsys_pm_domain_driver);
-- 
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 04/12] soc: mediatek: pm-domains: Add bus protection protocol

[Enric Balletbo i Serra]

From: Matthias Brugger <mbrugger@suse.com>

Bus protection will need to update more then one register
in infracfg. Add support for several operations.

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

 drivers/soc/mediatek/mtk-pm-domains.c | 44 +++++++++++++++++++--------
 1 file changed, 31 insertions(+), 13 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index db631dbaf2e3..52ac41ca871f 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -53,6 +53,8 @@
 #define PWR_STATUS_AUDIO		BIT(24)
 #define PWR_STATUS_USB			BIT(25)
 
+#define SPM_MAX_BUS_PROT_DATA		3
+
 struct scpsys_bus_prot_data {
 	u32 bus_prot_mask;
 	bool bus_prot_reg_update;
@@ -73,7 +75,7 @@ struct scpsys_domain_data {
 	u32 sram_pdn_bits;
 	u32 sram_pdn_ack_bits;
 	u8 caps;
-	const struct scpsys_bus_prot_data bp_infracfg;
+	const struct scpsys_bus_prot_data bp_infracfg[SPM_MAX_BUS_PROT_DATA];
 };
 
 struct scpsys_domain {
@@ -159,24 +161,40 @@ static int scpsys_sram_disable(struct scpsys_domain *pd, void __iomem *ctl_addr)
 
 static int scpsys_bus_protect_enable(struct scpsys_domain *pd)
 {
-	const struct scpsys_bus_prot_data *bp_data = &pd->data->bp_infracfg;
+	const struct scpsys_bus_prot_data *bpd = pd->data->bp_infracfg;
+	int i, ret;
 
-	if (!bp_data->bus_prot_mask)
-		return 0;
+	for (i = 0; i < SPM_MAX_BUS_PROT_DATA; i++) {
+		if (!bpd[i].bus_prot_mask)
+			break;
 
-	return mtk_infracfg_set_bus_protection(pd->infracfg, bp_data->bus_prot_mask,
-					       bp_data->bus_prot_reg_update);
+		ret = mtk_infracfg_set_bus_protection(pd->infracfg,
+						      bpd[i].bus_prot_mask,
+						      bpd[i].bus_prot_reg_update);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
 }
 
 static int scpsys_bus_protect_disable(struct scpsys_domain *pd)
 {
-	const struct scpsys_bus_prot_data *bp_data = &pd->data->bp_infracfg;
+	const struct scpsys_bus_prot_data *bpd = pd->data->bp_infracfg;
+	int i, ret;
+
+	for (i = 0; i < SPM_MAX_BUS_PROT_DATA; i++) {
+		if (!bpd[i].bus_prot_mask)
+			return 0;
 
-	if (!bp_data->bus_prot_mask)
-		return 0;
+		ret = mtk_infracfg_clear_bus_protection(pd->infracfg,
+							bpd[i].bus_prot_mask,
+							bpd[i].bus_prot_reg_update);
+		if (ret)
+			return ret;
+	}
 
-	return mtk_infracfg_clear_bus_protection(pd->infracfg, bp_data->bus_prot_mask,
-						 bp_data->bus_prot_reg_update);
+	return 0;
 }
 
 static int scpsys_power_on(struct generic_pm_domain *genpd)
@@ -482,7 +500,7 @@ static const struct scpsys_domain_data scpsys_domain_data_mt8173[] = {
 		.ctl_offs = SPM_DIS_PWR_CON,
 		.sram_pdn_bits = GENMASK(11, 8),
 		.sram_pdn_ack_bits = GENMASK(12, 12),
-		.bp_infracfg = {
+		.bp_infracfg[0] = {
 			.bus_prot_reg_update = true,
 			.bus_prot_mask = MT8173_TOP_AXI_PROT_EN_MM_M0 |
 				MT8173_TOP_AXI_PROT_EN_MM_M1,
@@ -524,7 +542,7 @@ static const struct scpsys_domain_data scpsys_domain_data_mt8173[] = {
 		.ctl_offs = SPM_MFG_PWR_CON,
 		.sram_pdn_bits = GENMASK(13, 8),
 		.sram_pdn_ack_bits = GENMASK(21, 16),
-		.bp_infracfg = {
+		.bp_infracfg[0] = {
 			.bus_prot_reg_update = true,
 			.bus_prot_mask = MT8173_TOP_AXI_PROT_EN_MFG_S |
 				MT8173_TOP_AXI_PROT_EN_MFG_M0 |
-- 
2.28.0

[PATCH 05/12] soc: mediatek: pm_domains: Make bus protection generic

[Enric Balletbo i Serra]

From: Matthias Brugger <mbrugger@suse.com>

Bus protection is not exclusively done by calling the infracfg misc driver.
Make the calls for setting and clearing the bus protection generic so
that we can use other blocks for it as well.

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

 drivers/soc/mediatek/mtk-infracfg.c   |  5 ---
 drivers/soc/mediatek/mtk-pm-domains.c | 54 +++++++++++++++++++++------
 include/linux/soc/mediatek/infracfg.h |  5 +++
 3 files changed, 48 insertions(+), 16 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-infracfg.c b/drivers/soc/mediatek/mtk-infracfg.c
index 341c7ac250e3..8871a524e023 100644
--- a/drivers/soc/mediatek/mtk-infracfg.c
+++ b/drivers/soc/mediatek/mtk-infracfg.c
@@ -12,11 +12,6 @@
 #define MTK_POLL_DELAY_US   10
 #define MTK_POLL_TIMEOUT    (jiffies_to_usecs(HZ))
 
-#define INFRA_TOPAXI_PROTECTEN		0x0220
-#define INFRA_TOPAXI_PROTECTSTA1	0x0228
-#define INFRA_TOPAXI_PROTECTEN_SET	0x0260
-#define INFRA_TOPAXI_PROTECTEN_CLR	0x0264
-
 /**
  * mtk_infracfg_set_bus_protection - enable bus protection
  * @regmap: The infracfg regmap
diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index 52ac41ca871f..f609c2d454fa 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -11,6 +11,7 @@
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/pm_domain.h>
+#include <linux/regmap.h>
 #include <linux/soc/mediatek/infracfg.h>
 
 #include <dt-bindings/power/mt8173-power.h>
@@ -159,18 +160,24 @@ static int scpsys_sram_disable(struct scpsys_domain *pd, void __iomem *ctl_addr)
 				  MTK_POLL_TIMEOUT);
 }
 
-static int scpsys_bus_protect_enable(struct scpsys_domain *pd)
+static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, struct regmap *regmap)
 {
-	const struct scpsys_bus_prot_data *bpd = pd->data->bp_infracfg;
 	int i, ret;
 
 	for (i = 0; i < SPM_MAX_BUS_PROT_DATA; i++) {
-		if (!bpd[i].bus_prot_mask)
+		u32 val, mask = bpd[i].bus_prot_mask;
+
+		if (!mask)
 			break;
 
-		ret = mtk_infracfg_set_bus_protection(pd->infracfg,
-						      bpd[i].bus_prot_mask,
-						      bpd[i].bus_prot_reg_update);
+		if (bpd[i].bus_prot_reg_update)
+			regmap_update_bits(regmap, INFRA_TOPAXI_PROTECTEN, mask, mask);
+		else
+			regmap_write(regmap, INFRA_TOPAXI_PROTECTEN_SET, mask);
+
+		ret = regmap_read_poll_timeout(regmap, INFRA_TOPAXI_PROTECTSTA1,
+					       val, (val & mask) == mask,
+					       MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
 		if (ret)
 			return ret;
 	}
@@ -178,18 +185,34 @@ static int scpsys_bus_protect_enable(struct scpsys_domain *pd)
 	return 0;
 }
 
-static int scpsys_bus_protect_disable(struct scpsys_domain *pd)
+static int scpsys_bus_protect_enable(struct scpsys_domain *pd)
 {
 	const struct scpsys_bus_prot_data *bpd = pd->data->bp_infracfg;
+	int ret;
+
+	ret = _scpsys_bus_protect_enable(bpd, pd->infracfg);
+	return ret;
+}
+
+static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
+				       struct regmap *regmap)
+{
 	int i, ret;
 
 	for (i = 0; i < SPM_MAX_BUS_PROT_DATA; i++) {
-		if (!bpd[i].bus_prot_mask)
+		u32 val, mask = bpd[i].bus_prot_mask;
+
+		if (!mask)
 			return 0;
 
-		ret = mtk_infracfg_clear_bus_protection(pd->infracfg,
-							bpd[i].bus_prot_mask,
-							bpd[i].bus_prot_reg_update);
+		if (bpd[i].bus_prot_reg_update)
+			regmap_update_bits(regmap, INFRA_TOPAXI_PROTECTEN, mask, 0);
+		else
+			regmap_write(regmap, INFRA_TOPAXI_PROTECTEN_CLR, mask);
+
+		ret = regmap_read_poll_timeout(regmap, INFRA_TOPAXI_PROTECTSTA1,
+					       val, !(val & mask),
+					       MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
 		if (ret)
 			return ret;
 	}
@@ -197,6 +220,15 @@ static int scpsys_bus_protect_disable(struct scpsys_domain *pd)
 	return 0;
 }
 
+static int scpsys_bus_protect_disable(struct scpsys_domain *pd)
+{
+	const struct scpsys_bus_prot_data *bpd = pd->data->bp_infracfg;
+	int ret;
+
+	ret = _scpsys_bus_protect_disable(bpd, pd->infracfg);
+	return ret;
+}
+
 static int scpsys_power_on(struct generic_pm_domain *genpd)
 {
 	struct scpsys_domain *pd = container_of(genpd, struct scpsys_domain, genpd);
diff --git a/include/linux/soc/mediatek/infracfg.h b/include/linux/soc/mediatek/infracfg.h
index fd25f0148566..f967d02cc2ff 100644
--- a/include/linux/soc/mediatek/infracfg.h
+++ b/include/linux/soc/mediatek/infracfg.h
@@ -32,6 +32,11 @@
 #define MT7622_TOP_AXI_PROT_EN_WB		(BIT(2) | BIT(6) | \
 						 BIT(7) | BIT(8))
 
+#define INFRA_TOPAXI_PROTECTEN		0x0220
+#define INFRA_TOPAXI_PROTECTSTA1	0x0228
+#define INFRA_TOPAXI_PROTECTEN_SET	0x0260
+#define INFRA_TOPAXI_PROTECTEN_CLR	0x0264
+
 int mtk_infracfg_set_bus_protection(struct regmap *infracfg, u32 mask,
 		bool reg_update);
 int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask,
-- 
2.28.0

[PATCH 06/12] soc: mediatek: pm-domains: Add SMI block as bus protection block

[Enric Balletbo i Serra]

From: Matthias Brugger <mbrugger@suse.com>

Apart from the infracfg block, the SMI block is used to enable the bus
protection for some power domains. Add support for this block.

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

 drivers/soc/mediatek/mtk-pm-domains.c | 64 ++++++++++++++++++++-------
 include/linux/soc/mediatek/infracfg.h |  6 +++
 2 files changed, 53 insertions(+), 17 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index f609c2d454fa..3aa430a60602 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -56,8 +56,25 @@
 
 #define SPM_MAX_BUS_PROT_DATA		3
 
+#define _BUS_PROT(_mask, _set, _clr, _sta, _update) {	\
+		.bus_prot_mask = (_mask),		\
+		.bus_prot_set = _set,			\
+		.bus_prot_clr = _clr,			\
+		.bus_prot_sta = _sta,			\
+		.bus_prot_reg_update = _update,		\
+	}
+
+#define BUS_PROT_WR(_mask, _set, _clr, _sta)		\
+		_BUS_PROT(_mask, _set, _clr, _sta, false)
+
+#define BUS_PROT_UPDATE(_mask, _set, _clr, _sta)		\
+		_BUS_PROT(_mask, _set, _clr, _sta, true)
+
 struct scpsys_bus_prot_data {
 	u32 bus_prot_mask;
+	u32 bus_prot_set;
+	u32 bus_prot_clr;
+	u32 bus_prot_sta;
 	bool bus_prot_reg_update;
 };
 
@@ -69,6 +86,7 @@ struct scpsys_bus_prot_data {
  * @sram_pdn_ack_bits: The mask for sram power control acked bits.
  * @caps: The flag for active wake-up action.
  * @bp_infracfg: bus protection for infracfg subsystem
+ * @bp_smi: bus protection for smi subsystem
  */
 struct scpsys_domain_data {
 	u32 sta_mask;
@@ -77,6 +95,7 @@ struct scpsys_domain_data {
 	u32 sram_pdn_ack_bits;
 	u8 caps;
 	const struct scpsys_bus_prot_data bp_infracfg[SPM_MAX_BUS_PROT_DATA];
+	const struct scpsys_bus_prot_data bp_smi[SPM_MAX_BUS_PROT_DATA];
 };
 
 struct scpsys_domain {
@@ -86,6 +105,7 @@ struct scpsys_domain {
 	int num_clks;
 	struct clk_bulk_data *clks;
 	struct regmap *infracfg;
+	struct regmap *smi;
 };
 
 struct scpsys_soc_data {
@@ -173,9 +193,9 @@ static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, st
 		if (bpd[i].bus_prot_reg_update)
 			regmap_update_bits(regmap, INFRA_TOPAXI_PROTECTEN, mask, mask);
 		else
-			regmap_write(regmap, INFRA_TOPAXI_PROTECTEN_SET, mask);
+			regmap_write(regmap, bpd[i].bus_prot_set, mask);
 
-		ret = regmap_read_poll_timeout(regmap, INFRA_TOPAXI_PROTECTSTA1,
+		ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
 					       val, (val & mask) == mask,
 					       MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
 		if (ret)
@@ -191,7 +211,11 @@ static int scpsys_bus_protect_enable(struct scpsys_domain *pd)
 	int ret;
 
 	ret = _scpsys_bus_protect_enable(bpd, pd->infracfg);
-	return ret;
+	if (ret)
+		return ret;
+
+	bpd = pd->data->bp_smi;
+	return _scpsys_bus_protect_enable(bpd, pd->smi);
 }
 
 static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
@@ -206,11 +230,11 @@ static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
 			return 0;
 
 		if (bpd[i].bus_prot_reg_update)
-			regmap_update_bits(regmap, INFRA_TOPAXI_PROTECTEN, mask, 0);
+			regmap_update_bits(regmap, bpd[i].bus_prot_set, mask, 0);
 		else
-			regmap_write(regmap, INFRA_TOPAXI_PROTECTEN_CLR, mask);
+			regmap_write(regmap, bpd[i].bus_prot_clr, mask);
 
-		ret = regmap_read_poll_timeout(regmap, INFRA_TOPAXI_PROTECTSTA1,
+		ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
 					       val, !(val & mask),
 					       MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
 		if (ret)
@@ -226,7 +250,11 @@ static int scpsys_bus_protect_disable(struct scpsys_domain *pd)
 	int ret;
 
 	ret = _scpsys_bus_protect_disable(bpd, pd->infracfg);
-	return ret;
+	if (ret)
+		return ret;
+
+	bpd = pd->data->bp_smi;
+	return _scpsys_bus_protect_disable(bpd, pd->smi);
 }
 
 static int scpsys_power_on(struct generic_pm_domain *genpd)
@@ -360,6 +388,10 @@ static int scpsys_add_one_domain(struct scpsys *scpsys, struct device_node *node
 	if (IS_ERR(pd->infracfg))
 		pd->infracfg = NULL;
 
+	pd->smi = syscon_regmap_lookup_by_phandle(node, "mediatek,smi");
+	if (IS_ERR(pd->smi))
+		pd->smi = NULL;
+
 	pd->num_clks = of_clk_get_parent_count(node);
 	if (pd->num_clks > 0) {
 		pd->clks = devm_kcalloc(scpsys->dev, pd->num_clks, sizeof(*pd->clks), GFP_KERNEL);
@@ -532,10 +564,9 @@ static const struct scpsys_domain_data scpsys_domain_data_mt8173[] = {
 		.ctl_offs = SPM_DIS_PWR_CON,
 		.sram_pdn_bits = GENMASK(11, 8),
 		.sram_pdn_ack_bits = GENMASK(12, 12),
-		.bp_infracfg[0] = {
-			.bus_prot_reg_update = true,
-			.bus_prot_mask = MT8173_TOP_AXI_PROT_EN_MM_M0 |
-				MT8173_TOP_AXI_PROT_EN_MM_M1,
+		.bp_infracfg = {
+			BUS_PROT_UPDATE_MT8173(MT8173_TOP_AXI_PROT_EN_MM_M0 |
+					       MT8173_TOP_AXI_PROT_EN_MM_M1),
 		},
 	},
 	[MT8173_POWER_DOMAIN_VENC_LT] = {
@@ -574,12 +605,11 @@ static const struct scpsys_domain_data scpsys_domain_data_mt8173[] = {
 		.ctl_offs = SPM_MFG_PWR_CON,
 		.sram_pdn_bits = GENMASK(13, 8),
 		.sram_pdn_ack_bits = GENMASK(21, 16),
-		.bp_infracfg[0] = {
-			.bus_prot_reg_update = true,
-			.bus_prot_mask = MT8173_TOP_AXI_PROT_EN_MFG_S |
-				MT8173_TOP_AXI_PROT_EN_MFG_M0 |
-				MT8173_TOP_AXI_PROT_EN_MFG_M1 |
-				MT8173_TOP_AXI_PROT_EN_MFG_SNOOP_OUT,
+		.bp_infracfg = {
+			BUS_PROT_UPDATE_MT8173(MT8173_TOP_AXI_PROT_EN_MFG_S |
+					       MT8173_TOP_AXI_PROT_EN_MFG_M0 |
+					       MT8173_TOP_AXI_PROT_EN_MFG_M1 |
+					       MT8173_TOP_AXI_PROT_EN_MFG_SNOOP_OUT),
 		},
 	},
 };
diff --git a/include/linux/soc/mediatek/infracfg.h b/include/linux/soc/mediatek/infracfg.h
index f967d02cc2ff..3f18cddffb44 100644
--- a/include/linux/soc/mediatek/infracfg.h
+++ b/include/linux/soc/mediatek/infracfg.h
@@ -37,6 +37,12 @@
 #define INFRA_TOPAXI_PROTECTEN_SET	0x0260
 #define INFRA_TOPAXI_PROTECTEN_CLR	0x0264
 
+#define BUS_PROT_UPDATE_MT8173(_mask)			\
+	BUS_PROT_UPDATE(_mask,				\
+			INFRA_TOPAXI_PROTECTEN,		\
+			INFRA_TOPAXI_PROTECTEN_CLR,	\
+			INFRA_TOPAXI_PROTECTSTA1)
+
 int mtk_infracfg_set_bus_protection(struct regmap *infracfg, u32 mask,
 		bool reg_update);
 int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask,
-- 
2.28.0

[PATCH 07/12] soc: mediatek: pm-domains: Add extra sram control

[Enric Balletbo i Serra]

From: Matthias Brugger <mbrugger@suse.com>

For some power domains like vpu_core on MT8183 whose sram need to do clock
and internal isolation while power on/off sram. We add a cap
"MTK_SCPD_SRAM_ISO" to judge if we need to do the extra sram isolation
control or not.

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

 drivers/soc/mediatek/mtk-pm-domains.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index 3aa430a60602..0802eccc3a0b 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -21,6 +21,7 @@
 
 #define MTK_SCPD_ACTIVE_WAKEUP		BIT(0)
 #define MTK_SCPD_FWAIT_SRAM		BIT(1)
+#define MTK_SCPD_SRAM_ISO		BIT(2)
 #define MTK_SCPD_CAPS(_scpd, _x)	((_scpd)->data->caps & (_x))
 
 #define SPM_VDE_PWR_CON			0x0210
@@ -42,6 +43,8 @@
 #define PWR_ON_BIT			BIT(2)
 #define PWR_ON_2ND_BIT			BIT(3)
 #define PWR_CLK_DIS_BIT			BIT(4)
+#define PWR_SRAM_CLKISO_BIT		BIT(5)
+#define PWR_SRAM_ISOINT_B_BIT		BIT(6)
 
 #define PWR_STATUS_DISP			BIT(3)
 #define PWR_STATUS_MFG			BIT(4)
@@ -162,6 +165,14 @@ static int scpsys_sram_enable(struct scpsys_domain *pd, void __iomem *ctl_addr)
 	if (ret < 0)
 		return ret;
 
+	if (MTK_SCPD_CAPS(pd, MTK_SCPD_SRAM_ISO))	{
+		val = readl(ctl_addr) | PWR_SRAM_ISOINT_B_BIT;
+		writel(val, ctl_addr);
+		udelay(1);
+		val &= ~PWR_SRAM_CLKISO_BIT;
+		writel(val, ctl_addr);
+	}
+
 	return 0;
 }
 
@@ -171,8 +182,15 @@ static int scpsys_sram_disable(struct scpsys_domain *pd, void __iomem *ctl_addr)
 	u32 val;
 	int tmp;
 
-	val = readl(ctl_addr);
-	val |= pd->data->sram_pdn_bits;
+	if (MTK_SCPD_CAPS(pd, MTK_SCPD_SRAM_ISO))	{
+		val = readl(ctl_addr) | PWR_SRAM_CLKISO_BIT;
+		writel(val, ctl_addr);
+		val &= ~PWR_SRAM_ISOINT_B_BIT;
+		writel(val, ctl_addr);
+		udelay(1);
+	}
+
+	val = readl(ctl_addr) | pd->data->sram_pdn_bits;
 	writel(val, ctl_addr);
 
 	/* Either wait until SRAM_PDN_ACK all 1 or 0 */
-- 
2.28.0

[PATCH 08/12] soc: mediatek: pm-domains: Add subsystem clocks

[Enric Balletbo i Serra]

From: Matthias Brugger <mbrugger@suse.com>

For the bus protection operations, some subsystem clocks need to be enabled
before releasing the protection. This patch identifies the subsystem clocks
by it's name.

Suggested-by: Weiyi Lu <weiyi.lu@mediatek.com>
[Adapted the patch to the mtk-pm-domains driver]
Signed-off-by: Matthias Brugger <mbrugger@suse.com>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

 drivers/soc/mediatek/mtk-pm-domains.c | 82 +++++++++++++++++++++++----
 1 file changed, 70 insertions(+), 12 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index 0802eccc3a0b..52a93a87e313 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -3,6 +3,7 @@
  * Copyright (c) 2020 Collabora Ltd.
  */
 #include <linux/clk.h>
+#include <linux/clk-provider.h>
 #include <linux/init.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
@@ -81,6 +82,8 @@ struct scpsys_bus_prot_data {
 	bool bus_prot_reg_update;
 };
 
+#define MAX_SUBSYS_CLKS 10
+
 /**
  * struct scpsys_domain_data - scp domain data for power on/off flow
  * @sta_mask: The mask for power on/off status bit.
@@ -107,6 +110,8 @@ struct scpsys_domain {
 	struct scpsys *scpsys;
 	int num_clks;
 	struct clk_bulk_data *clks;
+	int num_subsys_clks;
+	struct clk_bulk_data *subsys_clks;
 	struct regmap *infracfg;
 	struct regmap *smi;
 };
@@ -309,16 +314,22 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
 	val |= PWR_RST_B_BIT;
 	writel(val, ctl_addr);
 
+	ret = clk_bulk_enable(pd->num_subsys_clks, pd->subsys_clks);
+	if (ret)
+		goto err_pwr_ack;
+
 	ret = scpsys_sram_enable(pd, ctl_addr);
 	if (ret < 0)
-		goto err_pwr_ack;
+		goto err_sram;
 
 	ret = scpsys_bus_protect_disable(pd);
 	if (ret < 0)
-		goto err_pwr_ack;
+		goto err_sram;
 
 	return 0;
 
+err_sram:
+	clk_bulk_disable(pd->num_subsys_clks, pd->subsys_clks);
 err_pwr_ack:
 	clk_bulk_disable(pd->num_clks, pd->clks);
 	dev_err(scpsys->dev, "Failed to power on domain %s\n", genpd->name);
@@ -342,6 +353,8 @@ static int scpsys_power_off(struct generic_pm_domain *genpd)
 	if (ret < 0)
 		return ret;
 
+	clk_bulk_disable(pd->num_subsys_clks, pd->subsys_clks);
+
 	/* subsys power off */
 	val = readl(ctl_addr);
 	val |= PWR_ISO_BIT;
@@ -374,8 +387,11 @@ static int scpsys_add_one_domain(struct scpsys *scpsys, struct device_node *node
 {
 	const struct scpsys_domain_data *domain_data;
 	struct scpsys_domain *pd;
-	int i, ret;
+	int i, ret, num_clks;
 	u32 id;
+	int clk_ind = 0;
+	struct property *prop;
+	const char *clk_name;
 
 	ret = of_property_read_u32(node, "reg", &id);
 	if (ret) {
@@ -410,28 +426,60 @@ static int scpsys_add_one_domain(struct scpsys *scpsys, struct device_node *node
 	if (IS_ERR(pd->smi))
 		pd->smi = NULL;
 
-	pd->num_clks = of_clk_get_parent_count(node);
-	if (pd->num_clks > 0) {
+	num_clks = of_clk_get_parent_count(node);
+	if (num_clks > 0) {
+		/* Calculate number of subsys_clks */
+		of_property_for_each_string(node, "clock-names", prop, clk_name) {
+			char *subsys;
+
+			subsys = strchr(clk_name, '-');
+			if (subsys)
+				pd->num_subsys_clks++;
+			else
+				pd->num_clks++;
+		}
+
 		pd->clks = devm_kcalloc(scpsys->dev, pd->num_clks, sizeof(*pd->clks), GFP_KERNEL);
 		if (!pd->clks)
 			return -ENOMEM;
-	} else {
-		pd->num_clks = 0;
+
+		pd->subsys_clks = devm_kcalloc(scpsys->dev, pd->num_subsys_clks,
+					       sizeof(*pd->subsys_clks), GFP_KERNEL);
+		if (!pd->subsys_clks)
+			return -ENOMEM;
 	}
 
 	for (i = 0; i < pd->num_clks; i++) {
-		pd->clks[i].clk = of_clk_get(node, i);
-		if (IS_ERR(pd->clks[i].clk)) {
-			ret = PTR_ERR(pd->clks[i].clk);
+		struct clk *clk = of_clk_get(node, i);
+		if (IS_ERR(clk)) {
+			ret = PTR_ERR(clk);
 			dev_err(scpsys->dev, "%pOFn: failed to get clk at index %d: %d\n", node, i,
 				ret);
-			return ret;
+			goto err_put_clocks;
+		}
+
+		pd->clks[clk_ind++].clk = clk;
+	}
+
+	for (i = 0; i < pd->num_subsys_clks; i++) {
+		struct clk *clk = of_clk_get(node, i + clk_ind);
+		if (IS_ERR(clk)) {
+			ret = PTR_ERR(clk);
+			dev_err(scpsys->dev, "%pOFn: failed to get clk at index %d: %d\n", node,
+				i + clk_ind, ret);
+			goto err_put_subsys_clocks;
 		}
+
+		pd->subsys_clks[i].clk = clk;
 	}
 
+	ret = clk_bulk_prepare(pd->num_subsys_clks, pd->subsys_clks);
+	if (ret)
+		goto err_put_subsys_clocks;
+
 	ret = clk_bulk_prepare(pd->num_clks, pd->clks);
 	if (ret)
-		goto err_put_clocks;
+		goto err_unprepare_subsys_clocks;
 
 	/*
 	 * Initially turn on all domains to make the domains usable
@@ -456,6 +504,12 @@ static int scpsys_add_one_domain(struct scpsys *scpsys, struct device_node *node
 
 err_unprepare_clocks:
 	clk_bulk_unprepare(pd->num_clks, pd->clks);
+err_unprepare_subsys_clocks:
+	clk_bulk_unprepare(pd->num_subsys_clks, pd->subsys_clks);
+err_put_subsys_clocks:
+	clk_bulk_put(pd->num_subsys_clks, pd->subsys_clks);
+	devm_kfree(scpsys->dev, pd->subsys_clks);
+	pd->num_subsys_clks = 0;
 err_put_clocks:
 	clk_bulk_put(pd->num_clks, pd->clks);
 	devm_kfree(scpsys->dev, pd->clks);
@@ -537,6 +591,10 @@ static void scpsys_remove_one_domain(struct scpsys_domain *pd)
 	clk_bulk_unprepare(pd->num_clks, pd->clks);
 	clk_bulk_put(pd->num_clks, pd->clks);
 	pd->num_clks = 0;
+
+	clk_bulk_unprepare(pd->num_subsys_clks, pd->subsys_clks);
+	clk_bulk_put(pd->num_subsys_clks, pd->subsys_clks);
+	pd->num_subsys_clks = 0;
 }
 
 static void scpsys_domain_cleanup(struct scpsys *scpsys)
-- 
2.28.0

[PATCH 09/12] soc: mediatek: pm-domains: Allow bus protection to ignore clear ack

[Enric Balletbo i Serra]

From: Matthias Brugger <mbrugger@suse.com>

In some cases the hardware does not create an acknowledgment of the
bus protection clearing. Add a flag to the bus protection indicating
that a clear event will be ignored.

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

 drivers/soc/mediatek/mtk-pm-domains.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index 52a93a87e313..29e88adc8ea6 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -60,19 +60,23 @@
 
 #define SPM_MAX_BUS_PROT_DATA		3
 
-#define _BUS_PROT(_mask, _set, _clr, _sta, _update) {	\
-		.bus_prot_mask = (_mask),		\
-		.bus_prot_set = _set,			\
-		.bus_prot_clr = _clr,			\
-		.bus_prot_sta = _sta,			\
-		.bus_prot_reg_update = _update,		\
+#define _BUS_PROT(_mask, _set, _clr, _sta, _update, _ignore) {	\
+		.bus_prot_mask = (_mask),			\
+		.bus_prot_set = _set,				\
+		.bus_prot_clr = _clr,				\
+		.bus_prot_sta = _sta,				\
+		.bus_prot_reg_update = _update,			\
+		.ignore_clr_ack = _ignore,			\
 	}
 
-#define BUS_PROT_WR(_mask, _set, _clr, _sta)		\
-		_BUS_PROT(_mask, _set, _clr, _sta, false)
+#define BUS_PROT_WR(_mask, _set, _clr, _sta)			\
+		_BUS_PROT(_mask, _set, _clr, _sta, false, false)
+
+#define BUS_PROT_WR_IGN(_mask, _set, _clr, _sta)		\
+		_BUS_PROT(_mask, _set, _clr, _sta, false, true)
 
 #define BUS_PROT_UPDATE(_mask, _set, _clr, _sta)		\
-		_BUS_PROT(_mask, _set, _clr, _sta, true)
+		_BUS_PROT(_mask, _set, _clr, _sta, true, false)
 
 struct scpsys_bus_prot_data {
 	u32 bus_prot_mask;
@@ -80,6 +84,7 @@ struct scpsys_bus_prot_data {
 	u32 bus_prot_clr;
 	u32 bus_prot_sta;
 	bool bus_prot_reg_update;
+	bool ignore_clr_ack;
 };
 
 #define MAX_SUBSYS_CLKS 10
@@ -257,6 +262,9 @@ static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
 		else
 			regmap_write(regmap, bpd[i].bus_prot_clr, mask);
 
+		if (bpd[i].ignore_clr_ack)
+			continue;
+
 		ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
 					       val, !(val & mask),
 					       MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
-- 
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 11/12] soc: mediatek: pm-domains: Add support for mt8183

[Enric Balletbo i Serra]

From: Matthias Brugger <mbrugger@suse.com>

Add the needed board data to support mt8183 SoC.

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

 drivers/soc/mediatek/mtk-pm-domains.c | 162 ++++++++++++++++++++++++++
 include/linux/soc/mediatek/infracfg.h |  28 +++++
 2 files changed, 190 insertions(+)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index 29e88adc8ea6..aa434f616fee 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -16,6 +16,7 @@
 #include <linux/soc/mediatek/infracfg.h>
 
 #include <dt-bindings/power/mt8173-power.h>
+#include <dt-bindings/power/mt8183-power.h>
 
 #define MTK_POLL_DELAY_US   10
 #define MTK_POLL_TIMEOUT    USEC_PER_SEC
@@ -47,6 +48,7 @@
 #define PWR_SRAM_CLKISO_BIT		BIT(5)
 #define PWR_SRAM_ISOINT_B_BIT		BIT(6)
 
+#define PWR_STATUS_CONN			BIT(1)
 #define PWR_STATUS_DISP			BIT(3)
 #define PWR_STATUS_MFG			BIT(4)
 #define PWR_STATUS_ISP			BIT(5)
@@ -698,6 +700,155 @@ static const struct scpsys_domain_data scpsys_domain_data_mt8173[] = {
 	},
 };
 
+/*
+ * MT8183 power domain support
+ */
+static const struct scpsys_domain_data scpsys_domain_data_mt8183[] = {
+	[MT8183_POWER_DOMAIN_AUDIO] = {
+		.sta_mask = PWR_STATUS_AUDIO,
+		.ctl_offs = 0x0314,
+		.sram_pdn_bits = GENMASK(11, 8),
+		.sram_pdn_ack_bits = GENMASK(15, 12),
+	},
+	[MT8183_POWER_DOMAIN_CONN] = {
+		.sta_mask = PWR_STATUS_CONN,
+		.ctl_offs = 0x032c,
+		.sram_pdn_bits = 0,
+		.sram_pdn_ack_bits = 0,
+		.bp_infracfg = {
+			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_CONN, 0x2a0, 0x2a4, 0x228),
+		},
+	},
+	[MT8183_POWER_DOMAIN_MFG_ASYNC] = {
+		.sta_mask = PWR_STATUS_MFG_ASYNC,
+		.ctl_offs = 0x0334,
+		.sram_pdn_bits = 0,
+		.sram_pdn_ack_bits = 0,
+	},
+	[MT8183_POWER_DOMAIN_MFG] = {
+		.sta_mask = PWR_STATUS_MFG,
+		.ctl_offs = 0x0338,
+		.sram_pdn_bits = GENMASK(8, 8),
+		.sram_pdn_ack_bits = GENMASK(12, 12),
+	},
+	[MT8183_POWER_DOMAIN_MFG_CORE0] = {
+		.sta_mask = BIT(7),
+		.ctl_offs = 0x034c,
+		.sram_pdn_bits = GENMASK(8, 8),
+		.sram_pdn_ack_bits = GENMASK(12, 12),
+	},
+	[MT8183_POWER_DOMAIN_MFG_CORE1] = {
+		.sta_mask = BIT(20),
+		.ctl_offs = 0x0310,
+		.sram_pdn_bits = GENMASK(8, 8),
+		.sram_pdn_ack_bits = GENMASK(12, 12),
+	},
+	[MT8183_POWER_DOMAIN_MFG_2D] = {
+		.sta_mask = PWR_STATUS_MFG_2D,
+		.ctl_offs = 0x0348,
+		.sram_pdn_bits = GENMASK(8, 8),
+		.sram_pdn_ack_bits = GENMASK(12, 12),
+		.bp_infracfg = {
+			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_1_MFG, 0x2a8, 0x2ac, 0x258),
+			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MFG, 0x2a0, 0x2a4, 0x228),
+		},
+	},
+	[MT8183_POWER_DOMAIN_DISP] = {
+		.sta_mask = PWR_STATUS_DISP,
+		.ctl_offs = 0x030c,
+		.sram_pdn_bits = GENMASK(8, 8),
+		.sram_pdn_ack_bits = GENMASK(12, 12),
+		.bp_infracfg = {
+			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_1_DISP, 0x2a8, 0x2ac, 0x258),
+			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_DISP, 0x2a0, 0x2a4, 0x228),
+		},
+		.bp_smi = {
+			BUS_PROT_WR(MT8183_SMI_COMMON_SMI_CLAMP_DISP, 0x3c4, 0x3c8, 0x3c0),
+		},
+	},
+	[MT8183_POWER_DOMAIN_CAM] = {
+		.sta_mask = BIT(25),
+		.ctl_offs = 0x0344,
+		.sram_pdn_bits = GENMASK(9, 8),
+		.sram_pdn_ack_bits = GENMASK(13, 12),
+		.bp_infracfg = {
+			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MM_CAM, 0x2d4, 0x2d8, 0x2ec),
+			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_CAM, 0x2a0, 0x2a4, 0x228),
+			BUS_PROT_WR_IGN(MT8183_TOP_AXI_PROT_EN_MM_CAM_2ND, 0x2d4, 0x2d8, 0x2ec),
+		},
+		.bp_smi = {
+			BUS_PROT_WR(MT8183_SMI_COMMON_SMI_CLAMP_CAM, 0x3c4, 0x3c8, 0x3c0),
+		},
+	},
+	[MT8183_POWER_DOMAIN_ISP] = {
+		.sta_mask = PWR_STATUS_ISP,
+		.ctl_offs = 0x0308,
+		.sram_pdn_bits = GENMASK(9, 8),
+		.sram_pdn_ack_bits = GENMASK(13, 12),
+		.bp_infracfg = {
+			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MM_ISP, 0x2d4, 0x2d8, 0x2ec),
+			BUS_PROT_WR_IGN(MT8183_TOP_AXI_PROT_EN_MM_ISP_2ND, 0x2d4, 0x2d8, 0x2ec),
+		},
+		.bp_smi = {
+			BUS_PROT_WR(MT8183_SMI_COMMON_SMI_CLAMP_ISP, 0x3c4, 0x3c8, 0x3c0),
+		},
+	},
+	[MT8183_POWER_DOMAIN_VDEC] = {
+		.sta_mask = BIT(31),
+		.ctl_offs = 0x0300,
+		.sram_pdn_bits = GENMASK(8, 8),
+		.sram_pdn_ack_bits = GENMASK(12, 12),
+		.bp_smi = {
+			BUS_PROT_WR(MT8183_SMI_COMMON_SMI_CLAMP_VDEC, 0x3c4, 0x3c8, 0x3c0),
+		},
+	},
+	[MT8183_POWER_DOMAIN_VENC] = {
+		.sta_mask = PWR_STATUS_VENC,
+		.ctl_offs = 0x0304,
+		.sram_pdn_bits = GENMASK(11, 8),
+		.sram_pdn_ack_bits = GENMASK(15, 12),
+		.bp_smi = {
+			BUS_PROT_WR(MT8183_SMI_COMMON_SMI_CLAMP_VENC, 0x3c4, 0x3c8, 0x3c0),
+		},
+	},
+	[MT8183_POWER_DOMAIN_VPU_TOP] = {
+		.sta_mask = BIT(26),
+		.ctl_offs = 0x0324,
+		.sram_pdn_bits = GENMASK(8, 8),
+		.sram_pdn_ack_bits = GENMASK(12, 12),
+		.bp_infracfg = {
+			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MM_VPU_TOP, 0x2d4, 0x2d8, 0x2ec),
+			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_VPU_TOP, 0x2a0, 0x2a4, 0x228),
+			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MM_VPU_TOP_2ND, 0x2d4, 0x2d8, 0x2ec),
+		},
+		.bp_smi = {
+			BUS_PROT_WR(MT8183_SMI_COMMON_SMI_CLAMP_VPU_TOP, 0x3c4, 0x3c8, 0x3c0),
+		},
+	},
+	[MT8183_POWER_DOMAIN_VPU_CORE0] = {
+		.sta_mask = BIT(27),
+		.ctl_offs = 0x33c,
+		.sram_pdn_bits = GENMASK(11, 8),
+		.sram_pdn_ack_bits = GENMASK(13, 12),
+		.bp_infracfg = {
+			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MCU_VPU_CORE0, 0x2c4, 0x2c8, 0x2e4),
+			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MCU_VPU_CORE0_2ND, 0x2c4, 0x2c8, 0x2e4),
+		},
+		.caps = MTK_SCPD_SRAM_ISO,
+	},
+	[MT8183_POWER_DOMAIN_VPU_CORE1] = {
+		.sta_mask = BIT(28),
+		.ctl_offs = 0x0340,
+		.sram_pdn_bits = GENMASK(11, 8),
+		.sram_pdn_ack_bits = GENMASK(13, 12),
+		.bp_infracfg = {
+			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MCU_VPU_CORE1, 0x2c4, 0x2c8, 0x2e4),
+			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MCU_VPU_CORE1_2ND, 0x2c4, 0x2c8, 0x2e4),
+		},
+		.caps = MTK_SCPD_SRAM_ISO,
+	},
+};
+
 static const struct scpsys_soc_data mt8173_scpsys_data = {
 	.domains = scpsys_domain_data_mt8173,
 	.num_domains = ARRAY_SIZE(scpsys_domain_data_mt8173),
@@ -705,11 +856,22 @@ static const struct scpsys_soc_data mt8173_scpsys_data = {
 	.pwr_sta2nd_offs = SPM_PWR_STATUS_2ND,
 };
 
+static const struct scpsys_soc_data mt8183_scpsys_data = {
+	.domains = scpsys_domain_data_mt8183,
+	.num_domains = ARRAY_SIZE(scpsys_domain_data_mt8183),
+	.pwr_sta_offs = 0x0180,
+	.pwr_sta2nd_offs = 0x0184
+};
+
 static const struct of_device_id scpsys_of_match[] = {
 	{
 		.compatible = "mediatek,mt8173-power-controller",
 		.data = &mt8173_scpsys_data,
 	},
+	{
+		.compatible = "mediatek,mt8183-power-controller",
+		.data = &mt8183_scpsys_data,
+	},
 	{ }
 };
 
diff --git a/include/linux/soc/mediatek/infracfg.h b/include/linux/soc/mediatek/infracfg.h
index 3f18cddffb44..2913ede9d734 100644
--- a/include/linux/soc/mediatek/infracfg.h
+++ b/include/linux/soc/mediatek/infracfg.h
@@ -2,6 +2,34 @@
 #ifndef __SOC_MEDIATEK_INFRACFG_H
 #define __SOC_MEDIATEK_INFRACFG_H
 
+#define MT8183_TOP_AXI_PROT_EN_DISP			(BIT(10) | BIT(11))
+#define MT8183_TOP_AXI_PROT_EN_CONN			(BIT(13) | BIT(14))
+#define MT8183_TOP_AXI_PROT_EN_MFG			(BIT(21) | BIT(22))
+#define MT8183_TOP_AXI_PROT_EN_CAM			BIT(28)
+#define MT8183_TOP_AXI_PROT_EN_VPU_TOP			BIT(27)
+#define MT8183_TOP_AXI_PROT_EN_1_DISP			(BIT(16) | BIT(17))
+#define MT8183_TOP_AXI_PROT_EN_1_MFG			GENMASK(21, 19)
+#define MT8183_TOP_AXI_PROT_EN_MM_ISP			(BIT(3) | BIT(8))
+#define MT8183_TOP_AXI_PROT_EN_MM_ISP_2ND		BIT(10)
+#define MT8183_TOP_AXI_PROT_EN_MM_CAM			(BIT(4) | BIT(5) | \
+							 BIT(9) | BIT(13))
+#define MT8183_TOP_AXI_PROT_EN_MM_VPU_TOP		(GENMASK(9, 6) | \
+							 BIT(12))
+#define MT8183_TOP_AXI_PROT_EN_MM_VPU_TOP_2ND		(BIT(10) | BIT(11))
+#define MT8183_TOP_AXI_PROT_EN_MM_CAM_2ND		BIT(11)
+#define MT8183_TOP_AXI_PROT_EN_MCU_VPU_CORE0_2ND	(BIT(0) | BIT(2) | \
+							 BIT(4))
+#define MT8183_TOP_AXI_PROT_EN_MCU_VPU_CORE1_2ND	(BIT(1) | BIT(3) | \
+							 BIT(5))
+#define MT8183_TOP_AXI_PROT_EN_MCU_VPU_CORE0		BIT(6)
+#define MT8183_TOP_AXI_PROT_EN_MCU_VPU_CORE1		BIT(7)
+#define MT8183_SMI_COMMON_SMI_CLAMP_DISP		GENMASK(7, 0)
+#define MT8183_SMI_COMMON_SMI_CLAMP_VENC		BIT(1)
+#define MT8183_SMI_COMMON_SMI_CLAMP_ISP			BIT(2)
+#define MT8183_SMI_COMMON_SMI_CLAMP_CAM			(BIT(3) | BIT(4))
+#define MT8183_SMI_COMMON_SMI_CLAMP_VPU_TOP		(BIT(5) | BIT(6))
+#define MT8183_SMI_COMMON_SMI_CLAMP_VDEC		BIT(7)
+
 #define MT8173_TOP_AXI_PROT_EN_MCI_M2		BIT(0)
 #define MT8173_TOP_AXI_PROT_EN_MM_M0		BIT(1)
 #define MT8173_TOP_AXI_PROT_EN_MM_M1		BIT(2)
-- 
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 07/12] soc: mediatek: pm-domains: Add extra sram control

[Matthias Brugger]

On 10/09/2020 19:28, Enric Balletbo i Serra wrote:
> From: Matthias Brugger <mbrugger@suse.com>
> 
> For some power domains like vpu_core on MT8183 whose sram need to do clock
> and internal isolation while power on/off sram. We add a cap
> "MTK_SCPD_SRAM_ISO" to judge if we need to do the extra sram isolation
> control or not.
> 
> Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> 
>   drivers/soc/mediatek/mtk-pm-domains.c | 22 ++++++++++++++++++++--
>   1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
> index 3aa430a60602..0802eccc3a0b 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.c
> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> @@ -21,6 +21,7 @@
>   
>   #define MTK_SCPD_ACTIVE_WAKEUP		BIT(0)
>   #define MTK_SCPD_FWAIT_SRAM		BIT(1)
> +#define MTK_SCPD_SRAM_ISO		BIT(2)
>   #define MTK_SCPD_CAPS(_scpd, _x)	((_scpd)->data->caps & (_x))
>   
>   #define SPM_VDE_PWR_CON			0x0210
> @@ -42,6 +43,8 @@
>   #define PWR_ON_BIT			BIT(2)
>   #define PWR_ON_2ND_BIT			BIT(3)
>   #define PWR_CLK_DIS_BIT			BIT(4)
> +#define PWR_SRAM_CLKISO_BIT		BIT(5)
> +#define PWR_SRAM_ISOINT_B_BIT		BIT(6)
>   
>   #define PWR_STATUS_DISP			BIT(3)
>   #define PWR_STATUS_MFG			BIT(4)
> @@ -162,6 +165,14 @@ static int scpsys_sram_enable(struct scpsys_domain *pd, void __iomem *ctl_addr)
>   	if (ret < 0)
>   		return ret;
>   
> +	if (MTK_SCPD_CAPS(pd, MTK_SCPD_SRAM_ISO))	{
> +		val = readl(ctl_addr) | PWR_SRAM_ISOINT_B_BIT;
> +		writel(val, ctl_addr);
> +		udelay(1);
> +		val &= ~PWR_SRAM_CLKISO_BIT;
> +		writel(val, ctl_addr);
> +	}
> +
>   	return 0;
>   }
>   
> @@ -171,8 +182,15 @@ static int scpsys_sram_disable(struct scpsys_domain *pd, void __iomem *ctl_addr)
>   	u32 val;
>   	int tmp;
>   
> -	val = readl(ctl_addr);
> -	val |= pd->data->sram_pdn_bits;
> +	if (MTK_SCPD_CAPS(pd, MTK_SCPD_SRAM_ISO))	{
> +		val = readl(ctl_addr) | PWR_SRAM_CLKISO_BIT;
> +		writel(val, ctl_addr);
> +		val &= ~PWR_SRAM_ISOINT_B_BIT;
> +		writel(val, ctl_addr);
> +		udelay(1);
> +	}
> +
> +	val = readl(ctl_addr) | pd->data->sram_pdn_bits;

Nit, I'd prefer:
val = readl(ctl_addr);
val |= pd->data->sram_pdn_bits;


>   	writel(val, ctl_addr);
>   
>   	/* Either wait until SRAM_PDN_ACK all 1 or 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 11/12] soc: mediatek: pm-domains: Add support for mt8183

[Matthias Brugger]

On 10/09/2020 19:28, Enric Balletbo i Serra wrote:
> From: Matthias Brugger <mbrugger@suse.com>
> 
> Add the needed board data to support mt8183 SoC.
> 
> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> 
>   drivers/soc/mediatek/mtk-pm-domains.c | 162 ++++++++++++++++++++++++++
>   include/linux/soc/mediatek/infracfg.h |  28 +++++
>   2 files changed, 190 insertions(+)
> 
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
> index 29e88adc8ea6..aa434f616fee 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.c
> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
[...]
>   
> +/*
> + * MT8183 power domain support
> + */
> +static const struct scpsys_domain_data scpsys_domain_data_mt8183[] = {
> +	[MT8183_POWER_DOMAIN_AUDIO] = {
> +		.sta_mask = PWR_STATUS_AUDIO,
> +		.ctl_offs = 0x0314,
> +		.sram_pdn_bits = GENMASK(11, 8),
> +		.sram_pdn_ack_bits = GENMASK(15, 12),
> +	},
> +	[MT8183_POWER_DOMAIN_CONN] = {
> +		.sta_mask = PWR_STATUS_CONN,
> +		.ctl_offs = 0x032c,
> +		.sram_pdn_bits = 0,
> +		.sram_pdn_ack_bits = 0,
> +		.bp_infracfg = {
> +			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_CONN, 0x2a0, 0x2a4, 0x228),

We have repeating values triplets for set, clear and status register in infracfg 
and SMI.

Weiyi can you help to get names to this registers? I wasn't able to find 
anything in the datasheet.

Thanks!
Matthias

> +		},
> +	},
> +	[MT8183_POWER_DOMAIN_MFG_ASYNC] = {
> +		.sta_mask = PWR_STATUS_MFG_ASYNC,
> +		.ctl_offs = 0x0334,
> +		.sram_pdn_bits = 0,
> +		.sram_pdn_ack_bits = 0,
> +	},
> +	[MT8183_POWER_DOMAIN_MFG] = {
> +		.sta_mask = PWR_STATUS_MFG,
> +		.ctl_offs = 0x0338,
> +		.sram_pdn_bits = GENMASK(8, 8),
> +		.sram_pdn_ack_bits = GENMASK(12, 12),
> +	},
> +	[MT8183_POWER_DOMAIN_MFG_CORE0] = {
> +		.sta_mask = BIT(7),
> +		.ctl_offs = 0x034c,
> +		.sram_pdn_bits = GENMASK(8, 8),
> +		.sram_pdn_ack_bits = GENMASK(12, 12),
> +	},
> +	[MT8183_POWER_DOMAIN_MFG_CORE1] = {
> +		.sta_mask = BIT(20),
> +		.ctl_offs = 0x0310,
> +		.sram_pdn_bits = GENMASK(8, 8),
> +		.sram_pdn_ack_bits = GENMASK(12, 12),
> +	},
> +	[MT8183_POWER_DOMAIN_MFG_2D] = {
> +		.sta_mask = PWR_STATUS_MFG_2D,
> +		.ctl_offs = 0x0348,
> +		.sram_pdn_bits = GENMASK(8, 8),
> +		.sram_pdn_ack_bits = GENMASK(12, 12),
> +		.bp_infracfg = {
> +			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_1_MFG, 0x2a8, 0x2ac, 0x258),
> +			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MFG, 0x2a0, 0x2a4, 0x228),
> +		},
> +	},
> +	[MT8183_POWER_DOMAIN_DISP] = {
> +		.sta_mask = PWR_STATUS_DISP,
> +		.ctl_offs = 0x030c,
> +		.sram_pdn_bits = GENMASK(8, 8),
> +		.sram_pdn_ack_bits = GENMASK(12, 12),
> +		.bp_infracfg = {
> +			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_1_DISP, 0x2a8, 0x2ac, 0x258),
> +			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_DISP, 0x2a0, 0x2a4, 0x228),
> +		},
> +		.bp_smi = {
> +			BUS_PROT_WR(MT8183_SMI_COMMON_SMI_CLAMP_DISP, 0x3c4, 0x3c8, 0x3c0),
> +		},
> +	},
> +	[MT8183_POWER_DOMAIN_CAM] = {
> +		.sta_mask = BIT(25),
> +		.ctl_offs = 0x0344,
> +		.sram_pdn_bits = GENMASK(9, 8),
> +		.sram_pdn_ack_bits = GENMASK(13, 12),
> +		.bp_infracfg = {
> +			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MM_CAM, 0x2d4, 0x2d8, 0x2ec),
> +			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_CAM, 0x2a0, 0x2a4, 0x228),
> +			BUS_PROT_WR_IGN(MT8183_TOP_AXI_PROT_EN_MM_CAM_2ND, 0x2d4, 0x2d8, 0x2ec),
> +		},
> +		.bp_smi = {
> +			BUS_PROT_WR(MT8183_SMI_COMMON_SMI_CLAMP_CAM, 0x3c4, 0x3c8, 0x3c0),
> +		},
> +	},
> +	[MT8183_POWER_DOMAIN_ISP] = {
> +		.sta_mask = PWR_STATUS_ISP,
> +		.ctl_offs = 0x0308,
> +		.sram_pdn_bits = GENMASK(9, 8),
> +		.sram_pdn_ack_bits = GENMASK(13, 12),
> +		.bp_infracfg = {
> +			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MM_ISP, 0x2d4, 0x2d8, 0x2ec),
> +			BUS_PROT_WR_IGN(MT8183_TOP_AXI_PROT_EN_MM_ISP_2ND, 0x2d4, 0x2d8, 0x2ec),
> +		},
> +		.bp_smi = {
> +			BUS_PROT_WR(MT8183_SMI_COMMON_SMI_CLAMP_ISP, 0x3c4, 0x3c8, 0x3c0),
> +		},
> +	},
> +	[MT8183_POWER_DOMAIN_VDEC] = {
> +		.sta_mask = BIT(31),
> +		.ctl_offs = 0x0300,
> +		.sram_pdn_bits = GENMASK(8, 8),
> +		.sram_pdn_ack_bits = GENMASK(12, 12),
> +		.bp_smi = {
> +			BUS_PROT_WR(MT8183_SMI_COMMON_SMI_CLAMP_VDEC, 0x3c4, 0x3c8, 0x3c0),
> +		},
> +	},
> +	[MT8183_POWER_DOMAIN_VENC] = {
> +		.sta_mask = PWR_STATUS_VENC,
> +		.ctl_offs = 0x0304,
> +		.sram_pdn_bits = GENMASK(11, 8),
> +		.sram_pdn_ack_bits = GENMASK(15, 12),
> +		.bp_smi = {
> +			BUS_PROT_WR(MT8183_SMI_COMMON_SMI_CLAMP_VENC, 0x3c4, 0x3c8, 0x3c0),
> +		},
> +	},
> +	[MT8183_POWER_DOMAIN_VPU_TOP] = {
> +		.sta_mask = BIT(26),
> +		.ctl_offs = 0x0324,
> +		.sram_pdn_bits = GENMASK(8, 8),
> +		.sram_pdn_ack_bits = GENMASK(12, 12),
> +		.bp_infracfg = {
> +			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MM_VPU_TOP, 0x2d4, 0x2d8, 0x2ec),
> +			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_VPU_TOP, 0x2a0, 0x2a4, 0x228),
> +			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MM_VPU_TOP_2ND, 0x2d4, 0x2d8, 0x2ec),
> +		},
> +		.bp_smi = {
> +			BUS_PROT_WR(MT8183_SMI_COMMON_SMI_CLAMP_VPU_TOP, 0x3c4, 0x3c8, 0x3c0),
> +		},
> +	},
> +	[MT8183_POWER_DOMAIN_VPU_CORE0] = {
> +		.sta_mask = BIT(27),
> +		.ctl_offs = 0x33c,
> +		.sram_pdn_bits = GENMASK(11, 8),
> +		.sram_pdn_ack_bits = GENMASK(13, 12),
> +		.bp_infracfg = {
> +			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MCU_VPU_CORE0, 0x2c4, 0x2c8, 0x2e4),
> +			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MCU_VPU_CORE0_2ND, 0x2c4, 0x2c8, 0x2e4),
> +		},
> +		.caps = MTK_SCPD_SRAM_ISO,
> +	},
> +	[MT8183_POWER_DOMAIN_VPU_CORE1] = {
> +		.sta_mask = BIT(28),
> +		.ctl_offs = 0x0340,
> +		.sram_pdn_bits = GENMASK(11, 8),
> +		.sram_pdn_ack_bits = GENMASK(13, 12),
> +		.bp_infracfg = {
> +			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MCU_VPU_CORE1, 0x2c4, 0x2c8, 0x2e4),
> +			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MCU_VPU_CORE1_2ND, 0x2c4, 0x2c8, 0x2e4),
> +		},
> +		.caps = MTK_SCPD_SRAM_ISO,
> +	},
> +};
> +
>   static const struct scpsys_soc_data mt8173_scpsys_data = {
>   	.domains = scpsys_domain_data_mt8173,
>   	.num_domains = ARRAY_SIZE(scpsys_domain_data_mt8173),
> @@ -705,11 +856,22 @@ static const struct scpsys_soc_data mt8173_scpsys_data = {
>   	.pwr_sta2nd_offs = SPM_PWR_STATUS_2ND,
>   };
>   
> +static const struct scpsys_soc_data mt8183_scpsys_data = {
> +	.domains = scpsys_domain_data_mt8183,
> +	.num_domains = ARRAY_SIZE(scpsys_domain_data_mt8183),
> +	.pwr_sta_offs = 0x0180,
> +	.pwr_sta2nd_offs = 0x0184
> +};
> +
>   static const struct of_device_id scpsys_of_match[] = {
>   	{
>   		.compatible = "mediatek,mt8173-power-controller",
>   		.data = &mt8173_scpsys_data,
>   	},
> +	{
> +		.compatible = "mediatek,mt8183-power-controller",
> +		.data = &mt8183_scpsys_data,
> +	},
>   	{ }
>   };
>   
> diff --git a/include/linux/soc/mediatek/infracfg.h b/include/linux/soc/mediatek/infracfg.h
> index 3f18cddffb44..2913ede9d734 100644
> --- a/include/linux/soc/mediatek/infracfg.h
> +++ b/include/linux/soc/mediatek/infracfg.h
> @@ -2,6 +2,34 @@
>   #ifndef __SOC_MEDIATEK_INFRACFG_H
>   #define __SOC_MEDIATEK_INFRACFG_H
>   
> +#define MT8183_TOP_AXI_PROT_EN_DISP			(BIT(10) | BIT(11))
> +#define MT8183_TOP_AXI_PROT_EN_CONN			(BIT(13) | BIT(14))
> +#define MT8183_TOP_AXI_PROT_EN_MFG			(BIT(21) | BIT(22))
> +#define MT8183_TOP_AXI_PROT_EN_CAM			BIT(28)
> +#define MT8183_TOP_AXI_PROT_EN_VPU_TOP			BIT(27)
> +#define MT8183_TOP_AXI_PROT_EN_1_DISP			(BIT(16) | BIT(17))
> +#define MT8183_TOP_AXI_PROT_EN_1_MFG			GENMASK(21, 19)
> +#define MT8183_TOP_AXI_PROT_EN_MM_ISP			(BIT(3) | BIT(8))
> +#define MT8183_TOP_AXI_PROT_EN_MM_ISP_2ND		BIT(10)
> +#define MT8183_TOP_AXI_PROT_EN_MM_CAM			(BIT(4) | BIT(5) | \
> +							 BIT(9) | BIT(13))
> +#define MT8183_TOP_AXI_PROT_EN_MM_VPU_TOP		(GENMASK(9, 6) | \
> +							 BIT(12))
> +#define MT8183_TOP_AXI_PROT_EN_MM_VPU_TOP_2ND		(BIT(10) | BIT(11))
> +#define MT8183_TOP_AXI_PROT_EN_MM_CAM_2ND		BIT(11)
> +#define MT8183_TOP_AXI_PROT_EN_MCU_VPU_CORE0_2ND	(BIT(0) | BIT(2) | \
> +							 BIT(4))
> +#define MT8183_TOP_AXI_PROT_EN_MCU_VPU_CORE1_2ND	(BIT(1) | BIT(3) | \
> +							 BIT(5))
> +#define MT8183_TOP_AXI_PROT_EN_MCU_VPU_CORE0		BIT(6)
> +#define MT8183_TOP_AXI_PROT_EN_MCU_VPU_CORE1		BIT(7)
> +#define MT8183_SMI_COMMON_SMI_CLAMP_DISP		GENMASK(7, 0)
> +#define MT8183_SMI_COMMON_SMI_CLAMP_VENC		BIT(1)
> +#define MT8183_SMI_COMMON_SMI_CLAMP_ISP			BIT(2)
> +#define MT8183_SMI_COMMON_SMI_CLAMP_CAM			(BIT(3) | BIT(4))
> +#define MT8183_SMI_COMMON_SMI_CLAMP_VPU_TOP		(BIT(5) | BIT(6))
> +#define MT8183_SMI_COMMON_SMI_CLAMP_VDEC		BIT(7)
> +
>   #define MT8173_TOP_AXI_PROT_EN_MCI_M2		BIT(0)
>   #define MT8173_TOP_AXI_PROT_EN_MM_M0		BIT(1)
>   #define MT8173_TOP_AXI_PROT_EN_MM_M1		BIT(2)
> 

Re: [PATCH 11/12] soc: mediatek: pm-domains: Add support for mt8183

[Matthias Brugger]

On 16/09/2020 11:46, Matthias Brugger wrote:
> 
> 
> On 10/09/2020 19:28, Enric Balletbo i Serra wrote:
>> From: Matthias Brugger <mbrugger@suse.com>
>>
>> Add the needed board data to support mt8183 SoC.
>>
>> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> ---
>>
>>   drivers/soc/mediatek/mtk-pm-domains.c | 162 ++++++++++++++++++++++++++
>>   include/linux/soc/mediatek/infracfg.h |  28 +++++
>>   2 files changed, 190 insertions(+)
>>
>> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c 
>> b/drivers/soc/mediatek/mtk-pm-domains.c
>> index 29e88adc8ea6..aa434f616fee 100644
>> --- a/drivers/soc/mediatek/mtk-pm-domains.c
>> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> [...]
>> +/*
>> + * MT8183 power domain support
>> + */
>> +static const struct scpsys_domain_data scpsys_domain_data_mt8183[] = {
>> +    [MT8183_POWER_DOMAIN_AUDIO] = {
>> +        .sta_mask = PWR_STATUS_AUDIO,
>> +        .ctl_offs = 0x0314,
>> +        .sram_pdn_bits = GENMASK(11, 8),
>> +        .sram_pdn_ack_bits = GENMASK(15, 12),
>> +    },
>> +    [MT8183_POWER_DOMAIN_CONN] = {
>> +        .sta_mask = PWR_STATUS_CONN,
>> +        .ctl_offs = 0x032c,
>> +        .sram_pdn_bits = 0,
>> +        .sram_pdn_ack_bits = 0,
>> +        .bp_infracfg = {
>> +            BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_CONN, 0x2a0, 0x2a4, 0x228),
> 
> We have repeating values triplets for set, clear and status register in infracfg 
> and SMI.
> 
> Weiyi can you help to get names to this registers? I wasn't able to find 
> anything in the datasheet.

I think for the infracfg part I figured it out:

#define	INFRA_TOPAXI_PROTECTEN_SET	0x2a0
#define	INFRA_TOPAXI_PROTECTEN_CLR	0x2a4
#define	INFRA_TOPAXI_PROTECTEN_STA1	0x228

#define	INFRA_TOPAXI_PROTECTEN_1_SET	0x2a8
#define	INFRA_TOPAXI_PROTECTEN_1_CLR	0x2ac
#define	INFRA_TOPAXI_PROTECTEN_STA1_1	0x258

#define	INFRA_TOPAXI_PROTECTEN_MCU_SET	0x2d4
#define	INFRA_TOPAXI_PROTECTEN_MCU_CLR	0x2d8
#define	INFRA_TOPAXI_PROTECTEN_MM_STA1	0x2ec

Weiyi, can you still provide the register names for the SMI?

Thanks in advance!
Matthias

> 
> Thanks!
> Matthias
> 
>> +        },
>> +    },
>> +    [MT8183_POWER_DOMAIN_MFG_ASYNC] = {
>> +        .sta_mask = PWR_STATUS_MFG_ASYNC,
>> +        .ctl_offs = 0x0334,
>> +        .sram_pdn_bits = 0,
>> +        .sram_pdn_ack_bits = 0,
>> +    },
>> +    [MT8183_POWER_DOMAIN_MFG] = {
>> +        .sta_mask = PWR_STATUS_MFG,
>> +        .ctl_offs = 0x0338,
>> +        .sram_pdn_bits = GENMASK(8, 8),
>> +        .sram_pdn_ack_bits = GENMASK(12, 12),
>> +    },
>> +    [MT8183_POWER_DOMAIN_MFG_CORE0] = {
>> +        .sta_mask = BIT(7),
>> +        .ctl_offs = 0x034c,
>> +        .sram_pdn_bits = GENMASK(8, 8),
>> +        .sram_pdn_ack_bits = GENMASK(12, 12),
>> +    },
>> +    [MT8183_POWER_DOMAIN_MFG_CORE1] = {
>> +        .sta_mask = BIT(20),
>> +        .ctl_offs = 0x0310,
>> +        .sram_pdn_bits = GENMASK(8, 8),
>> +        .sram_pdn_ack_bits = GENMASK(12, 12),
>> +    },
>> +    [MT8183_POWER_DOMAIN_MFG_2D] = {
>> +        .sta_mask = PWR_STATUS_MFG_2D,
>> +        .ctl_offs = 0x0348,
>> +        .sram_pdn_bits = GENMASK(8, 8),
>> +        .sram_pdn_ack_bits = GENMASK(12, 12),
>> +        .bp_infracfg = {
>> +            BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_1_MFG, 0x2a8, 0x2ac, 0x258),
>> +            BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MFG, 0x2a0, 0x2a4, 0x228),
>> +        },
>> +    },
>> +    [MT8183_POWER_DOMAIN_DISP] = {
>> +        .sta_mask = PWR_STATUS_DISP,
>> +        .ctl_offs = 0x030c,
>> +        .sram_pdn_bits = GENMASK(8, 8),
>> +        .sram_pdn_ack_bits = GENMASK(12, 12),
>> +        .bp_infracfg = {
>> +            BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_1_DISP, 0x2a8, 0x2ac, 0x258),
>> +            BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_DISP, 0x2a0, 0x2a4, 0x228),
>> +        },
>> +        .bp_smi = {
>> +            BUS_PROT_WR(MT8183_SMI_COMMON_SMI_CLAMP_DISP, 0x3c4, 0x3c8, 0x3c0),
>> +        },
>> +    },
>> +    [MT8183_POWER_DOMAIN_CAM] = {
>> +        .sta_mask = BIT(25),
>> +        .ctl_offs = 0x0344,
>> +        .sram_pdn_bits = GENMASK(9, 8),
>> +        .sram_pdn_ack_bits = GENMASK(13, 12),
>> +        .bp_infracfg = {
>> +            BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MM_CAM, 0x2d4, 0x2d8, 0x2ec),
>> +            BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_CAM, 0x2a0, 0x2a4, 0x228),
>> +            BUS_PROT_WR_IGN(MT8183_TOP_AXI_PROT_EN_MM_CAM_2ND, 0x2d4, 0x2d8, 
>> 0x2ec),
>> +        },
>> +        .bp_smi = {
>> +            BUS_PROT_WR(MT8183_SMI_COMMON_SMI_CLAMP_CAM, 0x3c4, 0x3c8, 0x3c0),
>> +        },
>> +    },
>> +    [MT8183_POWER_DOMAIN_ISP] = {
>> +        .sta_mask = PWR_STATUS_ISP,
>> +        .ctl_offs = 0x0308,
>> +        .sram_pdn_bits = GENMASK(9, 8),
>> +        .sram_pdn_ack_bits = GENMASK(13, 12),
>> +        .bp_infracfg = {
>> +            BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MM_ISP, 0x2d4, 0x2d8, 0x2ec),
>> +            BUS_PROT_WR_IGN(MT8183_TOP_AXI_PROT_EN_MM_ISP_2ND, 0x2d4, 0x2d8, 
>> 0x2ec),
>> +        },
>> +        .bp_smi = {
>> +            BUS_PROT_WR(MT8183_SMI_COMMON_SMI_CLAMP_ISP, 0x3c4, 0x3c8, 0x3c0),
>> +        },
>> +    },
>> +    [MT8183_POWER_DOMAIN_VDEC] = {
>> +        .sta_mask = BIT(31),
>> +        .ctl_offs = 0x0300,
>> +        .sram_pdn_bits = GENMASK(8, 8),
>> +        .sram_pdn_ack_bits = GENMASK(12, 12),
>> +        .bp_smi = {
>> +            BUS_PROT_WR(MT8183_SMI_COMMON_SMI_CLAMP_VDEC, 0x3c4, 0x3c8, 0x3c0),
>> +        },
>> +    },
>> +    [MT8183_POWER_DOMAIN_VENC] = {
>> +        .sta_mask = PWR_STATUS_VENC,
>> +        .ctl_offs = 0x0304,
>> +        .sram_pdn_bits = GENMASK(11, 8),
>> +        .sram_pdn_ack_bits = GENMASK(15, 12),
>> +        .bp_smi = {
>> +            BUS_PROT_WR(MT8183_SMI_COMMON_SMI_CLAMP_VENC, 0x3c4, 0x3c8, 0x3c0),
>> +        },
>> +    },
>> +    [MT8183_POWER_DOMAIN_VPU_TOP] = {
>> +        .sta_mask = BIT(26),
>> +        .ctl_offs = 0x0324,
>> +        .sram_pdn_bits = GENMASK(8, 8),
>> +        .sram_pdn_ack_bits = GENMASK(12, 12),
>> +        .bp_infracfg = {
>> +            BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MM_VPU_TOP, 0x2d4, 0x2d8, 0x2ec),
>> +            BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_VPU_TOP, 0x2a0, 0x2a4, 0x228),
>> +            BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MM_VPU_TOP_2ND, 0x2d4, 0x2d8, 
>> 0x2ec),
>> +        },
>> +        .bp_smi = {
>> +            BUS_PROT_WR(MT8183_SMI_COMMON_SMI_CLAMP_VPU_TOP, 0x3c4, 0x3c8, 
>> 0x3c0),
>> +        },
>> +    },
>> +    [MT8183_POWER_DOMAIN_VPU_CORE0] = {
>> +        .sta_mask = BIT(27),
>> +        .ctl_offs = 0x33c,
>> +        .sram_pdn_bits = GENMASK(11, 8),
>> +        .sram_pdn_ack_bits = GENMASK(13, 12),
>> +        .bp_infracfg = {
>> +            BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MCU_VPU_CORE0, 0x2c4, 0x2c8, 
>> 0x2e4),
>> +            BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MCU_VPU_CORE0_2ND, 0x2c4, 
>> 0x2c8, 0x2e4),
>> +        },
>> +        .caps = MTK_SCPD_SRAM_ISO,
>> +    },
>> +    [MT8183_POWER_DOMAIN_VPU_CORE1] = {
>> +        .sta_mask = BIT(28),
>> +        .ctl_offs = 0x0340,
>> +        .sram_pdn_bits = GENMASK(11, 8),
>> +        .sram_pdn_ack_bits = GENMASK(13, 12),
>> +        .bp_infracfg = {
>> +            BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MCU_VPU_CORE1, 0x2c4, 0x2c8, 
>> 0x2e4),
>> +            BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MCU_VPU_CORE1_2ND, 0x2c4, 
>> 0x2c8, 0x2e4),
>> +        },
>> +        .caps = MTK_SCPD_SRAM_ISO,
>> +    },
>> +};
>> +
>>   static const struct scpsys_soc_data mt8173_scpsys_data = {
>>       .domains = scpsys_domain_data_mt8173,
>>       .num_domains = ARRAY_SIZE(scpsys_domain_data_mt8173),
>> @@ -705,11 +856,22 @@ static const struct scpsys_soc_data mt8173_scpsys_data = {
>>       .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND,
>>   };
>> +static const struct scpsys_soc_data mt8183_scpsys_data = {
>> +    .domains = scpsys_domain_data_mt8183,
>> +    .num_domains = ARRAY_SIZE(scpsys_domain_data_mt8183),
>> +    .pwr_sta_offs = 0x0180,
>> +    .pwr_sta2nd_offs = 0x0184
>> +};
>> +
>>   static const struct of_device_id scpsys_of_match[] = {
>>       {
>>           .compatible = "mediatek,mt8173-power-controller",
>>           .data = &mt8173_scpsys_data,
>>       },
>> +    {
>> +        .compatible = "mediatek,mt8183-power-controller",
>> +        .data = &mt8183_scpsys_data,
>> +    },
>>       { }
>>   };
>> diff --git a/include/linux/soc/mediatek/infracfg.h 
>> b/include/linux/soc/mediatek/infracfg.h
>> index 3f18cddffb44..2913ede9d734 100644
>> --- a/include/linux/soc/mediatek/infracfg.h
>> +++ b/include/linux/soc/mediatek/infracfg.h
>> @@ -2,6 +2,34 @@
>>   #ifndef __SOC_MEDIATEK_INFRACFG_H
>>   #define __SOC_MEDIATEK_INFRACFG_H
>> +#define MT8183_TOP_AXI_PROT_EN_DISP            (BIT(10) | BIT(11))
>> +#define MT8183_TOP_AXI_PROT_EN_CONN            (BIT(13) | BIT(14))
>> +#define MT8183_TOP_AXI_PROT_EN_MFG            (BIT(21) | BIT(22))
>> +#define MT8183_TOP_AXI_PROT_EN_CAM            BIT(28)
>> +#define MT8183_TOP_AXI_PROT_EN_VPU_TOP            BIT(27)
>> +#define MT8183_TOP_AXI_PROT_EN_1_DISP            (BIT(16) | BIT(17))
>> +#define MT8183_TOP_AXI_PROT_EN_1_MFG            GENMASK(21, 19)
>> +#define MT8183_TOP_AXI_PROT_EN_MM_ISP            (BIT(3) | BIT(8))
>> +#define MT8183_TOP_AXI_PROT_EN_MM_ISP_2ND        BIT(10)
>> +#define MT8183_TOP_AXI_PROT_EN_MM_CAM            (BIT(4) | BIT(5) | \
>> +                             BIT(9) | BIT(13))
>> +#define MT8183_TOP_AXI_PROT_EN_MM_VPU_TOP        (GENMASK(9, 6) | \
>> +                             BIT(12))
>> +#define MT8183_TOP_AXI_PROT_EN_MM_VPU_TOP_2ND        (BIT(10) | BIT(11))
>> +#define MT8183_TOP_AXI_PROT_EN_MM_CAM_2ND        BIT(11)
>> +#define MT8183_TOP_AXI_PROT_EN_MCU_VPU_CORE0_2ND    (BIT(0) | BIT(2) | \
>> +                             BIT(4))
>> +#define MT8183_TOP_AXI_PROT_EN_MCU_VPU_CORE1_2ND    (BIT(1) | BIT(3) | \
>> +                             BIT(5))
>> +#define MT8183_TOP_AXI_PROT_EN_MCU_VPU_CORE0        BIT(6)
>> +#define MT8183_TOP_AXI_PROT_EN_MCU_VPU_CORE1        BIT(7)
>> +#define MT8183_SMI_COMMON_SMI_CLAMP_DISP        GENMASK(7, 0)
>> +#define MT8183_SMI_COMMON_SMI_CLAMP_VENC        BIT(1)
>> +#define MT8183_SMI_COMMON_SMI_CLAMP_ISP            BIT(2)
>> +#define MT8183_SMI_COMMON_SMI_CLAMP_CAM            (BIT(3) | BIT(4))
>> +#define MT8183_SMI_COMMON_SMI_CLAMP_VPU_TOP        (BIT(5) | BIT(6))
>> +#define MT8183_SMI_COMMON_SMI_CLAMP_VDEC        BIT(7)
>> +
>>   #define MT8173_TOP_AXI_PROT_EN_MCI_M2        BIT(0)
>>   #define MT8173_TOP_AXI_PROT_EN_MM_M0        BIT(1)
>>   #define MT8173_TOP_AXI_PROT_EN_MM_M1        BIT(2)
>>

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 11/12] soc: mediatek: pm-domains: Add support for mt8183

[Hsin-Yi Wang]

On Wed, Sep 16, 2020 at 8:19 PM Matthias Brugger <matthias.bgg@gmail.com> wrote:
>
>
>
> On 16/09/2020 11:46, Matthias Brugger wrote:
> >
> >
> > On 10/09/2020 19:28, Enric Balletbo i Serra wrote:
> >> From: Matthias Brugger <mbrugger@suse.com>
> >>
> >> Add the needed board data to support mt8183 SoC.
> >>
> >> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
> >> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> >> ---
> >>
> >>   drivers/soc/mediatek/mtk-pm-domains.c | 162 ++++++++++++++++++++++++++
> >>   include/linux/soc/mediatek/infracfg.h |  28 +++++
> >>   2 files changed, 190 insertions(+)
> >>
> >> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c
> >> b/drivers/soc/mediatek/mtk-pm-domains.c
> >> index 29e88adc8ea6..aa434f616fee 100644
> >> --- a/drivers/soc/mediatek/mtk-pm-domains.c
> >> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> > [...]
> >> +/*
> >> + * MT8183 power domain support
> >> + */
> >> +static const struct scpsys_domain_data scpsys_domain_data_mt8183[] = {
> >> +    [MT8183_POWER_DOMAIN_AUDIO] = {
> >> +        .sta_mask = PWR_STATUS_AUDIO,
> >> +        .ctl_offs = 0x0314,
> >> +        .sram_pdn_bits = GENMASK(11, 8),
> >> +        .sram_pdn_ack_bits = GENMASK(15, 12),
> >> +    },
> >> +    [MT8183_POWER_DOMAIN_CONN] = {
> >> +        .sta_mask = PWR_STATUS_CONN,
> >> +        .ctl_offs = 0x032c,
> >> +        .sram_pdn_bits = 0,
> >> +        .sram_pdn_ack_bits = 0,
> >> +        .bp_infracfg = {
> >> +            BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_CONN, 0x2a0, 0x2a4, 0x228),
> >
> > We have repeating values triplets for set, clear and status register in infracfg
> > and SMI.
> >
> > Weiyi can you help to get names to this registers? I wasn't able to find
> > anything in the datasheet.
>
> I think for the infracfg part I figured it out:
>
> #define INFRA_TOPAXI_PROTECTEN_SET      0x2a0
> #define INFRA_TOPAXI_PROTECTEN_CLR      0x2a4
> #define INFRA_TOPAXI_PROTECTEN_STA1     0x228
>
> #define INFRA_TOPAXI_PROTECTEN_1_SET    0x2a8
> #define INFRA_TOPAXI_PROTECTEN_1_CLR    0x2ac
> #define INFRA_TOPAXI_PROTECTEN_STA1_1   0x258
>
> #define INFRA_TOPAXI_PROTECTEN_MCU_SET  0x2d4
> #define INFRA_TOPAXI_PROTECTEN_MCU_CLR  0x2d8
> #define INFRA_TOPAXI_PROTECTEN_MM_STA1  0x2ec
>
> Weiyi, can you still provide the register names for the SMI?
>
> Thanks in advance!
> Matthias
>
Hi Matthias,

SMI names are
#define SMI_COMMON_CLAMP_EN 0x3c0
#define SMI_COMMON_CLAMP_EN_SET 0x3c4
#define SMI_COMMON_CLAMP_EN_CLR 0x3c8

Thanks

Re: [PATCH 11/12] soc: mediatek: pm-domains: Add support for mt8183

[Enric Balletbo i Serra]

Hi Hsin-Yi and Matthias,

Hsin-Yi, many thanks to provide the register names.

On 25/9/20 9:37, Hsin-Yi Wang wrote:
> On Wed, Sep 16, 2020 at 8:19 PM Matthias Brugger <matthias.bgg@gmail.com> wrote:
>>
>>
>>
>> On 16/09/2020 11:46, Matthias Brugger wrote:
>>>
>>>
>>> On 10/09/2020 19:28, Enric Balletbo i Serra wrote:
>>>> From: Matthias Brugger <mbrugger@suse.com>
>>>>
>>>> Add the needed board data to support mt8183 SoC.
>>>>
>>>> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
>>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>>>> ---
>>>>
>>>>   drivers/soc/mediatek/mtk-pm-domains.c | 162 ++++++++++++++++++++++++++
>>>>   include/linux/soc/mediatek/infracfg.h |  28 +++++
>>>>   2 files changed, 190 insertions(+)
>>>>
>>>> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c
>>>> b/drivers/soc/mediatek/mtk-pm-domains.c
>>>> index 29e88adc8ea6..aa434f616fee 100644
>>>> --- a/drivers/soc/mediatek/mtk-pm-domains.c
>>>> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
>>> [...]
>>>> +/*
>>>> + * MT8183 power domain support
>>>> + */
>>>> +static const struct scpsys_domain_data scpsys_domain_data_mt8183[] = {
>>>> +    [MT8183_POWER_DOMAIN_AUDIO] = {
>>>> +        .sta_mask = PWR_STATUS_AUDIO,
>>>> +        .ctl_offs = 0x0314,
>>>> +        .sram_pdn_bits = GENMASK(11, 8),
>>>> +        .sram_pdn_ack_bits = GENMASK(15, 12),
>>>> +    },
>>>> +    [MT8183_POWER_DOMAIN_CONN] = {
>>>> +        .sta_mask = PWR_STATUS_CONN,
>>>> +        .ctl_offs = 0x032c,
>>>> +        .sram_pdn_bits = 0,
>>>> +        .sram_pdn_ack_bits = 0,
>>>> +        .bp_infracfg = {
>>>> +            BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_CONN, 0x2a0, 0x2a4, 0x228),
>>>
>>> We have repeating values triplets for set, clear and status register in infracfg
>>> and SMI.
>>>
>>> Weiyi can you help to get names to this registers? I wasn't able to find
>>> anything in the datasheet.
>>
>> I think for the infracfg part I figured it out:
>>
>> #define INFRA_TOPAXI_PROTECTEN_SET      0x2a0
>> #define INFRA_TOPAXI_PROTECTEN_CLR      0x2a4
>> #define INFRA_TOPAXI_PROTECTEN_STA1     0x228
>>
>> #define INFRA_TOPAXI_PROTECTEN_1_SET    0x2a8
>> #define INFRA_TOPAXI_PROTECTEN_1_CLR    0x2ac
>> #define INFRA_TOPAXI_PROTECTEN_STA1_1   0x258
>>
>> #define INFRA_TOPAXI_PROTECTEN_MCU_SET  0x2d4
>> #define INFRA_TOPAXI_PROTECTEN_MCU_CLR  0x2d8
>> #define INFRA_TOPAXI_PROTECTEN_MM_STA1  0x2ec
>>

I think this is SoC specific, right? So, I should add the MT8183_ prefix.

>> Weiyi, can you still provide the register names for the SMI?
>>
>> Thanks in advance!
>> Matthias
>>
> Hi Matthias,
> 
> SMI names are
> #define SMI_COMMON_CLAMP_EN 0x3c0
> #define SMI_COMMON_CLAMP_EN_SET 0x3c4
> #define SMI_COMMON_CLAMP_EN_CLR 0x3c8
> 

The same here, this is specific for MT8183, right?

Thanks,
  Enric

> Thanks
> 

Re: [PATCH 11/12] soc: mediatek: pm-domains: Add support for mt8183

[Matthias Brugger]

On 25/09/2020 10:21, Enric Balletbo i Serra wrote:
> Hi Hsin-Yi and Matthias,
> 
> Hsin-Yi, many thanks to provide the register names.
> 
> On 25/9/20 9:37, Hsin-Yi Wang wrote:
>> On Wed, Sep 16, 2020 at 8:19 PM Matthias Brugger <matthias.bgg@gmail.com> wrote:
>>>
>>>
>>>
>>> On 16/09/2020 11:46, Matthias Brugger wrote:
>>>>
>>>>
>>>> On 10/09/2020 19:28, Enric Balletbo i Serra wrote:
>>>>> From: Matthias Brugger <mbrugger@suse.com>
>>>>>
>>>>> Add the needed board data to support mt8183 SoC.
>>>>>
>>>>> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
>>>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>>>>> ---
>>>>>
>>>>>    drivers/soc/mediatek/mtk-pm-domains.c | 162 ++++++++++++++++++++++++++
>>>>>    include/linux/soc/mediatek/infracfg.h |  28 +++++
>>>>>    2 files changed, 190 insertions(+)
>>>>>
>>>>> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c
>>>>> b/drivers/soc/mediatek/mtk-pm-domains.c
>>>>> index 29e88adc8ea6..aa434f616fee 100644
>>>>> --- a/drivers/soc/mediatek/mtk-pm-domains.c
>>>>> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
>>>> [...]
>>>>> +/*
>>>>> + * MT8183 power domain support
>>>>> + */
>>>>> +static const struct scpsys_domain_data scpsys_domain_data_mt8183[] = {
>>>>> +    [MT8183_POWER_DOMAIN_AUDIO] = {
>>>>> +        .sta_mask = PWR_STATUS_AUDIO,
>>>>> +        .ctl_offs = 0x0314,
>>>>> +        .sram_pdn_bits = GENMASK(11, 8),
>>>>> +        .sram_pdn_ack_bits = GENMASK(15, 12),
>>>>> +    },
>>>>> +    [MT8183_POWER_DOMAIN_CONN] = {
>>>>> +        .sta_mask = PWR_STATUS_CONN,
>>>>> +        .ctl_offs = 0x032c,
>>>>> +        .sram_pdn_bits = 0,
>>>>> +        .sram_pdn_ack_bits = 0,
>>>>> +        .bp_infracfg = {
>>>>> +            BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_CONN, 0x2a0, 0x2a4, 0x228),
>>>>
>>>> We have repeating values triplets for set, clear and status register in infracfg
>>>> and SMI.
>>>>
>>>> Weiyi can you help to get names to this registers? I wasn't able to find
>>>> anything in the datasheet.
>>>
>>> I think for the infracfg part I figured it out:
>>>
>>> #define INFRA_TOPAXI_PROTECTEN_SET      0x2a0
>>> #define INFRA_TOPAXI_PROTECTEN_CLR      0x2a4
>>> #define INFRA_TOPAXI_PROTECTEN_STA1     0x228
>>>
>>> #define INFRA_TOPAXI_PROTECTEN_1_SET    0x2a8
>>> #define INFRA_TOPAXI_PROTECTEN_1_CLR    0x2ac
>>> #define INFRA_TOPAXI_PROTECTEN_STA1_1   0x258
>>>
>>> #define INFRA_TOPAXI_PROTECTEN_MCU_SET  0x2d4
>>> #define INFRA_TOPAXI_PROTECTEN_MCU_CLR  0x2d8
>>> #define INFRA_TOPAXI_PROTECTEN_MM_STA1  0x2ec

These three should be:
INFRA_TOPAXI_PROTECTEN_MM_SET 0x2d4
INFRA_TOPAXI_PROTECTEN_MM_CLR 0x2d8
INFRA_TOPAXI_PROTECTEN_MM_STA1 0x2ec

>>>
> 
> I think this is SoC specific, right? So, I should add the MT8183_ prefix.
> 

It seems like in newer SoCs infracfg register map has changed the layout for 
INFRA_TOPAXI_PROTECTEN_SET and INFRA_TOPAXI_PROTECTEN_CLR registers. Apart from 
that it got expanded to be able to use bus protection on more HW blocks.

So not sure if MT8183_ is the right prefix. Maybe we should just rename 
INFRA_TOPAXI_PROTECTEN_SET to something like INFRA_TOPAXI_PROTECTEN_SET_V2 and 
do the same for INFRA_TOPAXI_PROTECTEN_CLR

Regards,
Matthias

>>> Weiyi, can you still provide the register names for the SMI?
>>>
>>> Thanks in advance!
>>> Matthias
>>>
>> Hi Matthias,
>>
>> SMI names are
>> #define SMI_COMMON_CLAMP_EN 0x3c0
>> #define SMI_COMMON_CLAMP_EN_SET 0x3c4
>> #define SMI_COMMON_CLAMP_EN_CLR 0x3c8
>>
> 
> The same here, this is specific for MT8183, right?
> 
> Thanks,
>    Enric
> 
>> Thanks
>>
> 

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 02/12] soc: mediatek: Add MediaTek SCPSYS power domains

[Weiyi Lu]

On Thu, 2020-09-10 at 19:28 +0200, Enric Balletbo i Serra wrote:
> The System Control Processor System (SCPSYS) has several power management
> related tasks in the system. This driver implements support to handle
> the different power domains supported in order to meet high performance
> and low power requirements.
> 
> 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>
> ---
> 
>  drivers/soc/mediatek/Kconfig          |  13 +
>  drivers/soc/mediatek/Makefile         |   1 +
>  drivers/soc/mediatek/mtk-pm-domains.c | 626 ++++++++++++++++++++++++++
>  3 files changed, 640 insertions(+)
>  create mode 100644 drivers/soc/mediatek/mtk-pm-domains.c
> 
> diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
> index 59a56cd790ec..68d800f9e4a5 100644
> --- a/drivers/soc/mediatek/Kconfig
> +++ b/drivers/soc/mediatek/Kconfig
> @@ -44,6 +44,19 @@ config MTK_SCPSYS
>  	  Say yes here to add support for the MediaTek SCPSYS power domain
>  	  driver.
>  
> +config MTK_SCPSYS_PM_DOMAINS
> +	bool "MediaTek SCPSYS generic power domain"
> +	default ARCH_MEDIATEK
> +	depends on PM
> +	depends on MTK_INFRACFG
> +	select PM_GENERIC_DOMAINS
> +	select REGMAP
> +	help
> +	  Say y here to enable power domain support.
> +	  In order to meet high performance and low power requirements, the System
> +	  Control Processor System (SCPSYS) has several power management related
> +	  tasks in the system.
> +
>  config MTK_MMSYS
>  	bool "MediaTek MMSYS Support"
>  	default ARCH_MEDIATEK
> diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
> index 01f9f873634a..1e60fb4f89d4 100644
> --- a/drivers/soc/mediatek/Makefile
> +++ b/drivers/soc/mediatek/Makefile
> @@ -3,4 +3,5 @@ obj-$(CONFIG_MTK_CMDQ) += mtk-cmdq-helper.o
>  obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o
>  obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o
>  obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o
> +obj-$(CONFIG_MTK_SCPSYS_PM_DOMAINS) += mtk-pm-domains.o
>  obj-$(CONFIG_MTK_MMSYS) += mtk-mmsys.o
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
> new file mode 100644
> index 000000000000..db631dbaf2e3
> --- /dev/null
> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> @@ -0,0 +1,626 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2020 Collabora Ltd.
> + */
> +#include <linux/clk.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of_clk.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/soc/mediatek/infracfg.h>
> +
> +#include <dt-bindings/power/mt8173-power.h>
> +
> +#define MTK_POLL_DELAY_US   10
> +#define MTK_POLL_TIMEOUT    USEC_PER_SEC
> +
> +#define MTK_SCPD_ACTIVE_WAKEUP		BIT(0)
> +#define MTK_SCPD_FWAIT_SRAM		BIT(1)
> +#define MTK_SCPD_CAPS(_scpd, _x)	((_scpd)->data->caps & (_x))
> +
> +#define SPM_VDE_PWR_CON			0x0210
> +#define SPM_MFG_PWR_CON			0x0214
> +#define SPM_VEN_PWR_CON			0x0230
> +#define SPM_ISP_PWR_CON			0x0238
> +#define SPM_DIS_PWR_CON			0x023c
> +#define SPM_VEN2_PWR_CON		0x0298
> +#define SPM_AUDIO_PWR_CON		0x029c
> +#define SPM_MFG_2D_PWR_CON		0x02c0
> +#define SPM_MFG_ASYNC_PWR_CON		0x02c4
> +#define SPM_USB_PWR_CON			0x02cc
> +

If me, I'd choose to write directly into the data of each SoC now
because it's inconsistent on most MediatTek chips.

> +#define SPM_PWR_STATUS			0x060c
> +#define SPM_PWR_STATUS_2ND		0x0610
> +
> +#define PWR_RST_B_BIT			BIT(0)
> +#define PWR_ISO_BIT			BIT(1)
> +#define PWR_ON_BIT			BIT(2)
> +#define PWR_ON_2ND_BIT			BIT(3)
> +#define PWR_CLK_DIS_BIT			BIT(4)
> +
> +#define PWR_STATUS_DISP			BIT(3)
> +#define PWR_STATUS_MFG			BIT(4)
> +#define PWR_STATUS_ISP			BIT(5)
> +#define PWR_STATUS_VDEC			BIT(7)
> +#define PWR_STATUS_VENC_LT		BIT(20)
> +#define PWR_STATUS_VENC			BIT(21)
> +#define PWR_STATUS_MFG_2D		BIT(22)
> +#define PWR_STATUS_MFG_ASYNC		BIT(23)
> +#define PWR_STATUS_AUDIO		BIT(24)
> +#define PWR_STATUS_USB			BIT(25)
> +

same here for the status bits.

> +struct scpsys_bus_prot_data {
> +	u32 bus_prot_mask;
> +	bool bus_prot_reg_update;
> +};
> +
> +/**
> + * struct scpsys_domain_data - scp domain data for power on/off flow
> + * @sta_mask: The mask for power on/off status bit.
> + * @ctl_offs: The offset for main power control register.
> + * @sram_pdn_bits: The mask for sram power control bits.
> + * @sram_pdn_ack_bits: The mask for sram power control acked bits.
> + * @caps: The flag for active wake-up action.
> + * @bp_infracfg: bus protection for infracfg subsystem
> + */
> +struct scpsys_domain_data {
> +	u32 sta_mask;
> +	int ctl_offs;
> +	u32 sram_pdn_bits;
> +	u32 sram_pdn_ack_bits;
> +	u8 caps;
> +	const struct scpsys_bus_prot_data bp_infracfg;
> +};
> +
> +struct scpsys_domain {
> +	struct generic_pm_domain genpd;
> +	const struct scpsys_domain_data *data;
> +	struct scpsys *scpsys;
> +	int num_clks;
> +	struct clk_bulk_data *clks;
> +	struct regmap *infracfg;

Could we move
struct regmap *infracfg;
back to struct scpsys? It seems we need to set this property many times
under each power domain sub node in device tree?

> +};
> +
> +struct scpsys_soc_data {
> +	const struct scpsys_domain_data *domains;
> +	int num_domains;
> +	int pwr_sta_offs;
> +	int pwr_sta2nd_offs;
> +};
> +
> +struct scpsys {
> +	struct device *dev;
> +	void __iomem *base;
> +	const struct scpsys_soc_data *soc_data;
> +	struct genpd_onecell_data pd_data;
> +	struct generic_pm_domain *domains[];
> +};
> +
> +#define to_scpsys_domain(gpd) container_of(gpd, struct scpsys_domain, genpd)
> +
> +static int scpsys_domain_is_on(struct scpsys_domain *pd)
> +{
> +	struct scpsys *scpsys = pd->scpsys;
> +
> +	u32 status = readl(scpsys->base + scpsys->soc_data->pwr_sta_offs) & pd->data->sta_mask;
> +	u32 status2 = readl(scpsys->base + scpsys->soc_data->pwr_sta2nd_offs) & pd->data->sta_mask;
> +
> +	/*
> +	 * A domain is on when both status bits are set. If only one is set
> +	 * return an error. This happens while powering up a domain
> +	 */
> +
> +	if (status && status2)
> +		return true;
> +	if (!status && !status2)
> +		return false;
> +
> +	return -EINVAL;
> +}
> +
> +static int scpsys_sram_enable(struct scpsys_domain *pd, void __iomem *ctl_addr)
> +{
> +	u32 pdn_ack = pd->data->sram_pdn_ack_bits;
> +	u32 val;
> +	int tmp;
> +	int ret;
> +
> +	val = readl(ctl_addr);
> +	val &= ~pd->data->sram_pdn_bits;
> +	writel(val, ctl_addr);
> +
> +	/* Either wait until SRAM_PDN_ACK all 1 or 0 */
> +	ret = readl_poll_timeout(ctl_addr, tmp, (tmp & pdn_ack) == 0, MTK_POLL_DELAY_US,
> +				 MTK_POLL_TIMEOUT);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int scpsys_sram_disable(struct scpsys_domain *pd, void __iomem *ctl_addr)
> +{
> +	u32 pdn_ack = pd->data->sram_pdn_ack_bits;
> +	u32 val;
> +	int tmp;
> +
> +	val = readl(ctl_addr);
> +	val |= pd->data->sram_pdn_bits;
> +	writel(val, ctl_addr);
> +
> +	/* Either wait until SRAM_PDN_ACK all 1 or 0 */
> +	return readl_poll_timeout(ctl_addr, tmp, (tmp & pdn_ack) == pdn_ack, MTK_POLL_DELAY_US,
> +				  MTK_POLL_TIMEOUT);
> +}
> +
> +static int scpsys_bus_protect_enable(struct scpsys_domain *pd)
> +{
> +	const struct scpsys_bus_prot_data *bp_data = &pd->data->bp_infracfg;
> +
> +	if (!bp_data->bus_prot_mask)
> +		return 0;
> +
> +	return mtk_infracfg_set_bus_protection(pd->infracfg, bp_data->bus_prot_mask,
> +					       bp_data->bus_prot_reg_update);
> +}
> +
> +static int scpsys_bus_protect_disable(struct scpsys_domain *pd)
> +{
> +	const struct scpsys_bus_prot_data *bp_data = &pd->data->bp_infracfg;
> +
> +	if (!bp_data->bus_prot_mask)
> +		return 0;
> +
> +	return mtk_infracfg_clear_bus_protection(pd->infracfg, bp_data->bus_prot_mask,
> +						 bp_data->bus_prot_reg_update);
> +}
> +
> +static int scpsys_power_on(struct generic_pm_domain *genpd)
> +{
> +	struct scpsys_domain *pd = container_of(genpd, struct scpsys_domain, genpd);
> +	struct scpsys *scpsys = pd->scpsys;
> +	void __iomem *ctl_addr = scpsys->base + pd->data->ctl_offs;
> +	int ret, tmp;
> +	u32 val;
> +
> +	ret = clk_bulk_enable(pd->num_clks, pd->clks);
> +	if (ret)
> +		return ret;
> +
> +	/* subsys power on */
> +	val = readl(ctl_addr);
> +	val |= PWR_ON_BIT;
> +	writel(val, ctl_addr);
> +	val |= PWR_ON_2ND_BIT;
> +	writel(val, ctl_addr);
> +
> +	/* wait until PWR_ACK = 1 */
> +	ret = readx_poll_timeout(scpsys_domain_is_on, pd, tmp, tmp > 0, MTK_POLL_DELAY_US,
> +				 MTK_POLL_TIMEOUT);
> +	if (ret < 0)
> +		goto err_pwr_ack;
> +
> +	val &= ~PWR_CLK_DIS_BIT;
> +	writel(val, ctl_addr);
> +
> +	val &= ~PWR_ISO_BIT;
> +	writel(val, ctl_addr);
> +
> +	val |= PWR_RST_B_BIT;
> +	writel(val, ctl_addr);
> +
> +	ret = scpsys_sram_enable(pd, ctl_addr);
> +	if (ret < 0)
> +		goto err_pwr_ack;
> +
> +	ret = scpsys_bus_protect_disable(pd);
> +	if (ret < 0)
> +		goto err_pwr_ack;
> +
> +	return 0;
> +
> +err_pwr_ack:
> +	clk_bulk_disable(pd->num_clks, pd->clks);
> +	dev_err(scpsys->dev, "Failed to power on domain %s\n", genpd->name);
> +
> +	return ret;
> +}
> +
> +static int scpsys_power_off(struct generic_pm_domain *genpd)
> +{
> +	struct scpsys_domain *pd = container_of(genpd, struct scpsys_domain, genpd);
> +	struct scpsys *scpsys = pd->scpsys;
> +	void __iomem *ctl_addr = scpsys->base + pd->data->ctl_offs;
> +	int ret, tmp;
> +	u32 val;
> +
> +	ret = scpsys_bus_protect_enable(pd);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = scpsys_sram_disable(pd, ctl_addr);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* subsys power off */
> +	val = readl(ctl_addr);
> +	val |= PWR_ISO_BIT;
> +	writel(val, ctl_addr);
> +
> +	val &= ~PWR_RST_B_BIT;
> +	writel(val, ctl_addr);
> +
> +	val |= PWR_CLK_DIS_BIT;
> +	writel(val, ctl_addr);
> +
> +	val &= ~PWR_ON_BIT;
> +	writel(val, ctl_addr);
> +
> +	val &= ~PWR_ON_2ND_BIT;
> +	writel(val, ctl_addr);
> +
> +	/* wait until PWR_ACK = 0 */
> +	ret = readx_poll_timeout(scpsys_domain_is_on, pd, tmp, tmp == 0, MTK_POLL_DELAY_US,
> +				 MTK_POLL_TIMEOUT);
> +	if (ret < 0)
> +		return ret;
> +
> +	clk_bulk_disable(pd->num_clks, pd->clks);
> +
> +	return 0;
> +}
> +
> +static int scpsys_add_one_domain(struct scpsys *scpsys, struct device_node *node)
> +{
> +	const struct scpsys_domain_data *domain_data;
> +	struct scpsys_domain *pd;
> +	int i, ret;
> +	u32 id;
> +
> +	ret = of_property_read_u32(node, "reg", &id);
> +	if (ret) {
> +		dev_err(scpsys->dev, "%pOFn: failed to retrieve domain id from reg: %d\n", node,
> +			ret);
> +		return -EINVAL;
> +	}
> +
> +	if (id >= scpsys->soc_data->num_domains) {
> +		dev_err(scpsys->dev, "%pOFn: invalid domain id %d\n", node, id);
> +		return -EINVAL;
> +	}
> +
> +	domain_data = &scpsys->soc_data->domains[id];
> +	if (!domain_data) {
> +		dev_err(scpsys->dev, "%pOFn: undefined domain id %d\n", node, id);
> +		return -EINVAL;
> +	}
> +
> +	pd = devm_kzalloc(scpsys->dev, sizeof(*pd), GFP_KERNEL);
> +	if (!pd)
> +		return -ENOMEM;
> +
> +	pd->data = domain_data;
> +	pd->scpsys = scpsys;
> +
> +	pd->infracfg = syscon_regmap_lookup_by_phandle(node, "mediatek,infracfg");
> +	if (IS_ERR(pd->infracfg))
> +		pd->infracfg = NULL;
> +
> +	pd->num_clks = of_clk_get_parent_count(node);
> +	if (pd->num_clks > 0) {
> +		pd->clks = devm_kcalloc(scpsys->dev, pd->num_clks, sizeof(*pd->clks), GFP_KERNEL);
> +		if (!pd->clks)
> +			return -ENOMEM;
> +	} else {
> +		pd->num_clks = 0;
> +	}
> +
> +	for (i = 0; i < pd->num_clks; i++) {
> +		pd->clks[i].clk = of_clk_get(node, i);

Is it possible to have a better way that we could use of_clk_bulk_get()?

> +		if (IS_ERR(pd->clks[i].clk)) {
> +			ret = PTR_ERR(pd->clks[i].clk);
> +			dev_err(scpsys->dev, "%pOFn: failed to get clk at index %d: %d\n", node, i,
> +				ret);
> +			return ret;
> +		}
> +	}
> +
> +	ret = clk_bulk_prepare(pd->num_clks, pd->clks);
> +	if (ret)
> +		goto err_put_clocks;
> +
> +	/*
> +	 * Initially turn on all domains to make the domains usable
> +	 * with !CONFIG_PM and to get the hardware in sync with the
> +	 * software.  The unused domains will be switched off during
> +	 * late_init time.
> +	 */
> +	ret = scpsys_power_on(&pd->genpd);
> +	if (ret < 0) {
> +		dev_err(scpsys->dev, "failed to power on domain %pOFN with error %d\n", node, ret);
> +		goto err_unprepare_clocks;
> +	}
> +
> +	pd->genpd.name = node->name;
> +	pd->genpd.power_off = scpsys_power_off;
> +	pd->genpd.power_on = scpsys_power_on;
> +
> +	pm_genpd_init(&pd->genpd, NULL, false);
> +
> +	scpsys->domains[id] = &pd->genpd;
> +	return 0;
> +
> +err_unprepare_clocks:
> +	clk_bulk_unprepare(pd->num_clks, pd->clks);
> +err_put_clocks:
> +	clk_bulk_put(pd->num_clks, pd->clks);
> +	devm_kfree(scpsys->dev, pd->clks);
> +	pd->num_clks = 0;
> +	return ret;
> +}
> +
> +static int scpsys_add_subdomain(struct scpsys *scpsys, struct device_node *parent)
> +{
> +	struct device_node *child;
> +	struct generic_pm_domain *child_pd, *parent_pd;
> +	int ret;
> +
> +	for_each_child_of_node(parent, child) {
> +		u32 id;
> +
> +		ret = of_property_read_u32(parent, "reg", &id);
> +		if (ret) {
> +			dev_err(scpsys->dev, "%pOFn: failed to get parent domain id: %d\n", child,
> +				ret);
> +			goto err_put_node;
> +		}
> +		parent_pd = scpsys->pd_data.domains[id];
> +

nit. Could we move parent outside(in front of) the loop?

> +		ret = scpsys_add_one_domain(scpsys, child);
> +		if (ret) {
> +			dev_err(scpsys->dev, "error adding power domain for %pOFn: %d\n", child,
> +				ret);
> +			goto err_put_node;
> +		}
> +
> +		ret = of_property_read_u32(child, "reg", &id);
> +		if (ret) {
> +			dev_err(scpsys->dev, "%pOFn: failed to get child domain id: %d\n", child,
> +				ret);
> +			goto err_put_node;
> +		}
> +		child_pd = scpsys->pd_data.domains[id];
> +
> +		ret = pm_genpd_add_subdomain(parent_pd, child_pd);
> +		if (ret) {
> +			dev_err(scpsys->dev, "failed to add %s subdomain to parent %s\n",
> +				child_pd->name, parent_pd->name);
> +			goto err_put_node;
> +		} else {
> +			dev_dbg(scpsys->dev, "%s add subdomain: %s\n", parent_pd->name,
> +				child_pd->name);
> +		}
> +
> +		/* recursive call to add all subdomains */
> +		ret = scpsys_add_subdomain(scpsys, child);
> +		if (ret)
> +			goto err_put_node;
> +	}
> +
> +	return 0;
> +
> +err_put_node:
> +	of_node_put(child);
> +	return ret;
> +}
> +
> +static void scpsys_remove_one_domain(struct scpsys_domain *pd)
> +{
> +	int ret;
> +
> +	/*
> +	 * We're in the error cleanup already, so we only complain,
> +	 * but won't emit another error on top of the original one.
> +	 */
> +	ret = pm_genpd_remove(&pd->genpd);
> +	if (ret < 0)
> +		dev_err(pd->scpsys->dev,
> +			"failed to remove domain '%s' : %d - state may be inconsistent\n",
> +			pd->genpd.name, ret);
> +
> +	scpsys_power_off(&pd->genpd);
> +
> +	clk_bulk_unprepare(pd->num_clks, pd->clks);
> +	clk_bulk_put(pd->num_clks, pd->clks);
> +	pd->num_clks = 0;
> +}
> +
> +static void scpsys_domain_cleanup(struct scpsys *scpsys)
> +{
> +	struct generic_pm_domain *genpd;
> +	struct scpsys_domain *pd;
> +	int i;
> +
> +	for (i = scpsys->pd_data.num_domains - 1; i >= 0; i--) {
> +		genpd = scpsys->pd_data.domains[i];
> +		if (genpd) {
> +			pd = to_scpsys_domain(genpd);
> +			scpsys_remove_one_domain(pd);
> +		}
> +	}
> +}
> +
> +/*
> + * MT8173 power domain support
> + */
> +
> +static const struct scpsys_domain_data scpsys_domain_data_mt8173[] = {
> +	[MT8173_POWER_DOMAIN_VDEC] = {
> +		.sta_mask = PWR_STATUS_VDEC,
> +		.ctl_offs = SPM_VDE_PWR_CON,
> +		.sram_pdn_bits = GENMASK(11, 8),
> +		.sram_pdn_ack_bits = GENMASK(12, 12),
> +	},
> +	[MT8173_POWER_DOMAIN_VENC] = {
> +		.sta_mask = PWR_STATUS_VENC,
> +		.ctl_offs = SPM_VEN_PWR_CON,
> +		.sram_pdn_bits = GENMASK(11, 8),
> +		.sram_pdn_ack_bits = GENMASK(15, 12),
> +	},
> +	[MT8173_POWER_DOMAIN_ISP] = {
> +		.sta_mask = PWR_STATUS_ISP,
> +		.ctl_offs = SPM_ISP_PWR_CON,
> +		.sram_pdn_bits = GENMASK(11, 8),
> +		.sram_pdn_ack_bits = GENMASK(13, 12),
> +	},
> +	[MT8173_POWER_DOMAIN_MM] = {
> +		.sta_mask = PWR_STATUS_DISP,
> +		.ctl_offs = SPM_DIS_PWR_CON,
> +		.sram_pdn_bits = GENMASK(11, 8),
> +		.sram_pdn_ack_bits = GENMASK(12, 12),
> +		.bp_infracfg = {
> +			.bus_prot_reg_update = true,
> +			.bus_prot_mask = MT8173_TOP_AXI_PROT_EN_MM_M0 |
> +				MT8173_TOP_AXI_PROT_EN_MM_M1,
> +		},
> +	},
> +	[MT8173_POWER_DOMAIN_VENC_LT] = {
> +		.sta_mask = PWR_STATUS_VENC_LT,
> +		.ctl_offs = SPM_VEN2_PWR_CON,
> +		.sram_pdn_bits = GENMASK(11, 8),
> +		.sram_pdn_ack_bits = GENMASK(15, 12),
> +	},
> +	[MT8173_POWER_DOMAIN_AUDIO] = {
> +		.sta_mask = PWR_STATUS_AUDIO,
> +		.ctl_offs = SPM_AUDIO_PWR_CON,
> +		.sram_pdn_bits = GENMASK(11, 8),
> +		.sram_pdn_ack_bits = GENMASK(15, 12),
> +	},
> +	[MT8173_POWER_DOMAIN_USB] = {
> +		.sta_mask = PWR_STATUS_USB,
> +		.ctl_offs = SPM_USB_PWR_CON,
> +		.sram_pdn_bits = GENMASK(11, 8),
> +		.sram_pdn_ack_bits = GENMASK(15, 12),
> +		.caps = MTK_SCPD_ACTIVE_WAKEUP,
> +	},
> +	[MT8173_POWER_DOMAIN_MFG_ASYNC] = {
> +		.sta_mask = PWR_STATUS_MFG_ASYNC,
> +		.ctl_offs = SPM_MFG_ASYNC_PWR_CON,
> +		.sram_pdn_bits = GENMASK(11, 8),
> +		.sram_pdn_ack_bits = 0,
> +	},
> +	[MT8173_POWER_DOMAIN_MFG_2D] = {
> +		.sta_mask = PWR_STATUS_MFG_2D,
> +		.ctl_offs = SPM_MFG_2D_PWR_CON,
> +		.sram_pdn_bits = GENMASK(11, 8),
> +		.sram_pdn_ack_bits = GENMASK(13, 12),
> +	},
> +	[MT8173_POWER_DOMAIN_MFG] = {
> +		.sta_mask = PWR_STATUS_MFG,
> +		.ctl_offs = SPM_MFG_PWR_CON,
> +		.sram_pdn_bits = GENMASK(13, 8),
> +		.sram_pdn_ack_bits = GENMASK(21, 16),
> +		.bp_infracfg = {
> +			.bus_prot_reg_update = true,
> +			.bus_prot_mask = MT8173_TOP_AXI_PROT_EN_MFG_S |
> +				MT8173_TOP_AXI_PROT_EN_MFG_M0 |
> +				MT8173_TOP_AXI_PROT_EN_MFG_M1 |
> +				MT8173_TOP_AXI_PROT_EN_MFG_SNOOP_OUT,
> +		},
> +	},
> +};
> +
> +static const struct scpsys_soc_data mt8173_scpsys_data = {
> +	.domains = scpsys_domain_data_mt8173,
> +	.num_domains = ARRAY_SIZE(scpsys_domain_data_mt8173),
> +	.pwr_sta_offs = SPM_PWR_STATUS,
> +	.pwr_sta2nd_offs = SPM_PWR_STATUS_2ND,
> +};
> +
> +static const struct of_device_id scpsys_of_match[] = {
> +	{
> +		.compatible = "mediatek,mt8173-power-controller",
> +		.data = &mt8173_scpsys_data,
> +	},
> +	{ }
> +};
> +
> +static int scpsys_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *np = dev->of_node;
> +	const struct scpsys_soc_data *soc;
> +	struct device_node *node;
> +	struct scpsys *scpsys;
> +	struct resource *res;
> +	int ret;
> +
> +	soc = of_device_get_match_data(&pdev->dev);
> +	if (!soc) {
> +		dev_err(&pdev->dev, "no power controller data\n");
> +		return -EINVAL;
> +	}
> +
> +	scpsys = devm_kzalloc(dev, struct_size(scpsys, domains, soc->num_domains), GFP_KERNEL);
> +	if (!scpsys)
> +		return -ENOMEM;
> +
> +	scpsys->dev = dev;
> +	scpsys->soc_data = soc;
> +
> +	scpsys->pd_data.domains = scpsys->domains;
> +	scpsys->pd_data.num_domains = soc->num_domains;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	scpsys->base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(scpsys->base))
> +		return -ENODEV;
> +
> +	ret = -ENODEV;
> +	for_each_available_child_of_node(np, node) {
> +		ret = scpsys_add_one_domain(scpsys, node);
> +		if (ret) {
> +			dev_err(dev, "failed to handle node %pOFN: %d\n", node, ret);
> +			of_node_put(node);
> +			goto err_cleanup_domains;
> +		}
> +
> +		ret = scpsys_add_subdomain(scpsys, node);
> +		if (ret) {
> +			dev_err(dev, "failed to add subdomain node %pOFn: %d\n", node, ret);
> +			of_node_put(node);
> +			goto err_cleanup_domains;
> +		}
> +	}
> +
> +	if (ret) {
> +		dev_dbg(dev, "no power domains present\n");
> +		return ret;
> +	}
> +
> +	ret = of_genpd_add_provider_onecell(np, &scpsys->pd_data);
> +	if (ret) {
> +		dev_err(dev, "failed to add provider: %d\n", ret);
> +		goto err_cleanup_domains;
> +	}
> +
> +	return 0;
> +
> +err_cleanup_domains:
> +	scpsys_domain_cleanup(scpsys);
> +	return ret;
> +}
> +
> +static struct platform_driver scpsys_pm_domain_driver = {
> +	.probe = scpsys_probe,
> +	.driver = {
> +		.name = "mtk-power-controller",
> +		.suppress_bind_attrs = true,
> +		.of_match_table = scpsys_of_match,
> +	},
> +};
> +builtin_platform_driver(scpsys_pm_domain_driver);

Re: [PATCH 02/12] soc: mediatek: Add MediaTek SCPSYS power domains

[Matthias Brugger]

On 25/09/2020 12:25, Weiyi Lu wrote:
> On Thu, 2020-09-10 at 19:28 +0200, Enric Balletbo i Serra wrote:
>> The System Control Processor System (SCPSYS) has several power management
>> related tasks in the system. This driver implements support to handle
>> the different power domains supported in order to meet high performance
>> and low power requirements.
>>
>> 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>
>> ---
>>
>>   drivers/soc/mediatek/Kconfig          |  13 +
>>   drivers/soc/mediatek/Makefile         |   1 +
>>   drivers/soc/mediatek/mtk-pm-domains.c | 626 ++++++++++++++++++++++++++
>>   3 files changed, 640 insertions(+)
>>   create mode 100644 drivers/soc/mediatek/mtk-pm-domains.c
>>
>> diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
>> index 59a56cd790ec..68d800f9e4a5 100644
>> --- a/drivers/soc/mediatek/Kconfig
>> +++ b/drivers/soc/mediatek/Kconfig
>> @@ -44,6 +44,19 @@ config MTK_SCPSYS
>>   	  Say yes here to add support for the MediaTek SCPSYS power domain
>>   	  driver.
>>   
>> +config MTK_SCPSYS_PM_DOMAINS
>> +	bool "MediaTek SCPSYS generic power domain"
>> +	default ARCH_MEDIATEK
>> +	depends on PM
>> +	depends on MTK_INFRACFG
>> +	select PM_GENERIC_DOMAINS
>> +	select REGMAP
>> +	help
>> +	  Say y here to enable power domain support.
>> +	  In order to meet high performance and low power requirements, the System
>> +	  Control Processor System (SCPSYS) has several power management related
>> +	  tasks in the system.
>> +
>>   config MTK_MMSYS
>>   	bool "MediaTek MMSYS Support"
>>   	default ARCH_MEDIATEK
>> diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
>> index 01f9f873634a..1e60fb4f89d4 100644
>> --- a/drivers/soc/mediatek/Makefile
>> +++ b/drivers/soc/mediatek/Makefile
>> @@ -3,4 +3,5 @@ obj-$(CONFIG_MTK_CMDQ) += mtk-cmdq-helper.o
>>   obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o
>>   obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o
>>   obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o
>> +obj-$(CONFIG_MTK_SCPSYS_PM_DOMAINS) += mtk-pm-domains.o
>>   obj-$(CONFIG_MTK_MMSYS) += mtk-mmsys.o
>> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
>> new file mode 100644
>> index 000000000000..db631dbaf2e3
>> --- /dev/null
>> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
>> @@ -0,0 +1,626 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2020 Collabora Ltd.
>> + */
>> +#include <linux/clk.h>
>> +#include <linux/init.h>
>> +#include <linux/io.h>
>> +#include <linux/iopoll.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/of_clk.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pm_domain.h>
>> +#include <linux/soc/mediatek/infracfg.h>
>> +
>> +#include <dt-bindings/power/mt8173-power.h>
>> +
>> +#define MTK_POLL_DELAY_US   10
>> +#define MTK_POLL_TIMEOUT    USEC_PER_SEC
>> +
>> +#define MTK_SCPD_ACTIVE_WAKEUP		BIT(0)
>> +#define MTK_SCPD_FWAIT_SRAM		BIT(1)
>> +#define MTK_SCPD_CAPS(_scpd, _x)	((_scpd)->data->caps & (_x))
>> +
>> +#define SPM_VDE_PWR_CON			0x0210
>> +#define SPM_MFG_PWR_CON			0x0214
>> +#define SPM_VEN_PWR_CON			0x0230
>> +#define SPM_ISP_PWR_CON			0x0238
>> +#define SPM_DIS_PWR_CON			0x023c
>> +#define SPM_VEN2_PWR_CON		0x0298
>> +#define SPM_AUDIO_PWR_CON		0x029c
>> +#define SPM_MFG_2D_PWR_CON		0x02c0
>> +#define SPM_MFG_ASYNC_PWR_CON		0x02c4
>> +#define SPM_USB_PWR_CON			0x02cc
>> +
> 
> If me, I'd choose to write directly into the data of each SoC now
> because it's inconsistent on most MediatTek chips.
> 

What do you mean with "write directly into the data of each SoC"?

>> +#define SPM_PWR_STATUS			0x060c
>> +#define SPM_PWR_STATUS_2ND		0x0610
>> +
>> +#define PWR_RST_B_BIT			BIT(0)
>> +#define PWR_ISO_BIT			BIT(1)
>> +#define PWR_ON_BIT			BIT(2)
>> +#define PWR_ON_2ND_BIT			BIT(3)
>> +#define PWR_CLK_DIS_BIT			BIT(4)
>> +
>> +#define PWR_STATUS_DISP			BIT(3)
>> +#define PWR_STATUS_MFG			BIT(4)
>> +#define PWR_STATUS_ISP			BIT(5)
>> +#define PWR_STATUS_VDEC			BIT(7)
>> +#define PWR_STATUS_VENC_LT		BIT(20)
>> +#define PWR_STATUS_VENC			BIT(21)
>> +#define PWR_STATUS_MFG_2D		BIT(22)
>> +#define PWR_STATUS_MFG_ASYNC		BIT(23)
>> +#define PWR_STATUS_AUDIO		BIT(24)
>> +#define PWR_STATUS_USB			BIT(25)
>> +
> 
> same here for the status bits.
> 

Up to now we have litte collisions in the different defines. Not sure what you 
propose.

>> +struct scpsys_bus_prot_data {
>> +	u32 bus_prot_mask;
>> +	bool bus_prot_reg_update;
>> +};
>> +
>> +/**
>> + * struct scpsys_domain_data - scp domain data for power on/off flow
>> + * @sta_mask: The mask for power on/off status bit.
>> + * @ctl_offs: The offset for main power control register.
>> + * @sram_pdn_bits: The mask for sram power control bits.
>> + * @sram_pdn_ack_bits: The mask for sram power control acked bits.
>> + * @caps: The flag for active wake-up action.
>> + * @bp_infracfg: bus protection for infracfg subsystem
>> + */
>> +struct scpsys_domain_data {
>> +	u32 sta_mask;
>> +	int ctl_offs;
>> +	u32 sram_pdn_bits;
>> +	u32 sram_pdn_ack_bits;
>> +	u8 caps;
>> +	const struct scpsys_bus_prot_data bp_infracfg;
>> +};
>> +
>> +struct scpsys_domain {
>> +	struct generic_pm_domain genpd;
>> +	const struct scpsys_domain_data *data;
>> +	struct scpsys *scpsys;
>> +	int num_clks;
>> +	struct clk_bulk_data *clks;
>> +	struct regmap *infracfg;
> 
> Could we move
> struct regmap *infracfg;
> back to struct scpsys? It seems we need to set this property many times
> under each power domain sub node in device tree?
> 

With the new design only the power domain that needs infracfg/smi will actually 
get a handle to the regmap.
That allows us to describe the HW better in device tree.

This is done once when we load the driver I don't see why this should be an 
issue (if you were referring to the fact that it takes longer do this several 
times instead of once). So I think it is a good trade-off.

>> +};
>> +
>> +struct scpsys_soc_data {
>> +	const struct scpsys_domain_data *domains;
>> +	int num_domains;
>> +	int pwr_sta_offs;
>> +	int pwr_sta2nd_offs;
>> +};
>> +
>> +struct scpsys {
>> +	struct device *dev;
>> +	void __iomem *base;
>> +	const struct scpsys_soc_data *soc_data;
>> +	struct genpd_onecell_data pd_data;
>> +	struct generic_pm_domain *domains[];
>> +};
>> +
>> +#define to_scpsys_domain(gpd) container_of(gpd, struct scpsys_domain, genpd)
>> +
>> +static int scpsys_domain_is_on(struct scpsys_domain *pd)
>> +{
>> +	struct scpsys *scpsys = pd->scpsys;
>> +
>> +	u32 status = readl(scpsys->base + scpsys->soc_data->pwr_sta_offs) & pd->data->sta_mask;
>> +	u32 status2 = readl(scpsys->base + scpsys->soc_data->pwr_sta2nd_offs) & pd->data->sta_mask;
>> +
>> +	/*
>> +	 * A domain is on when both status bits are set. If only one is set
>> +	 * return an error. This happens while powering up a domain
>> +	 */
>> +
>> +	if (status && status2)
>> +		return true;
>> +	if (!status && !status2)
>> +		return false;
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +static int scpsys_sram_enable(struct scpsys_domain *pd, void __iomem *ctl_addr)
>> +{
>> +	u32 pdn_ack = pd->data->sram_pdn_ack_bits;
>> +	u32 val;
>> +	int tmp;
>> +	int ret;
>> +
>> +	val = readl(ctl_addr);
>> +	val &= ~pd->data->sram_pdn_bits;
>> +	writel(val, ctl_addr);
>> +
>> +	/* Either wait until SRAM_PDN_ACK all 1 or 0 */
>> +	ret = readl_poll_timeout(ctl_addr, tmp, (tmp & pdn_ack) == 0, MTK_POLL_DELAY_US,
>> +				 MTK_POLL_TIMEOUT);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return 0;
>> +}
>> +
>> +static int scpsys_sram_disable(struct scpsys_domain *pd, void __iomem *ctl_addr)
>> +{
>> +	u32 pdn_ack = pd->data->sram_pdn_ack_bits;
>> +	u32 val;
>> +	int tmp;
>> +
>> +	val = readl(ctl_addr);
>> +	val |= pd->data->sram_pdn_bits;
>> +	writel(val, ctl_addr);
>> +
>> +	/* Either wait until SRAM_PDN_ACK all 1 or 0 */
>> +	return readl_poll_timeout(ctl_addr, tmp, (tmp & pdn_ack) == pdn_ack, MTK_POLL_DELAY_US,
>> +				  MTK_POLL_TIMEOUT);
>> +}
>> +
>> +static int scpsys_bus_protect_enable(struct scpsys_domain *pd)
>> +{
>> +	const struct scpsys_bus_prot_data *bp_data = &pd->data->bp_infracfg;
>> +
>> +	if (!bp_data->bus_prot_mask)
>> +		return 0;
>> +
>> +	return mtk_infracfg_set_bus_protection(pd->infracfg, bp_data->bus_prot_mask,
>> +					       bp_data->bus_prot_reg_update);
>> +}
>> +
>> +static int scpsys_bus_protect_disable(struct scpsys_domain *pd)
>> +{
>> +	const struct scpsys_bus_prot_data *bp_data = &pd->data->bp_infracfg;
>> +
>> +	if (!bp_data->bus_prot_mask)
>> +		return 0;
>> +
>> +	return mtk_infracfg_clear_bus_protection(pd->infracfg, bp_data->bus_prot_mask,
>> +						 bp_data->bus_prot_reg_update);
>> +}
>> +
>> +static int scpsys_power_on(struct generic_pm_domain *genpd)
>> +{
>> +	struct scpsys_domain *pd = container_of(genpd, struct scpsys_domain, genpd);
>> +	struct scpsys *scpsys = pd->scpsys;
>> +	void __iomem *ctl_addr = scpsys->base + pd->data->ctl_offs;
>> +	int ret, tmp;
>> +	u32 val;
>> +
>> +	ret = clk_bulk_enable(pd->num_clks, pd->clks);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* subsys power on */
>> +	val = readl(ctl_addr);
>> +	val |= PWR_ON_BIT;
>> +	writel(val, ctl_addr);
>> +	val |= PWR_ON_2ND_BIT;
>> +	writel(val, ctl_addr);
>> +
>> +	/* wait until PWR_ACK = 1 */
>> +	ret = readx_poll_timeout(scpsys_domain_is_on, pd, tmp, tmp > 0, MTK_POLL_DELAY_US,
>> +				 MTK_POLL_TIMEOUT);
>> +	if (ret < 0)
>> +		goto err_pwr_ack;
>> +
>> +	val &= ~PWR_CLK_DIS_BIT;
>> +	writel(val, ctl_addr);
>> +
>> +	val &= ~PWR_ISO_BIT;
>> +	writel(val, ctl_addr);
>> +
>> +	val |= PWR_RST_B_BIT;
>> +	writel(val, ctl_addr);
>> +
>> +	ret = scpsys_sram_enable(pd, ctl_addr);
>> +	if (ret < 0)
>> +		goto err_pwr_ack;
>> +
>> +	ret = scpsys_bus_protect_disable(pd);
>> +	if (ret < 0)
>> +		goto err_pwr_ack;
>> +
>> +	return 0;
>> +
>> +err_pwr_ack:
>> +	clk_bulk_disable(pd->num_clks, pd->clks);
>> +	dev_err(scpsys->dev, "Failed to power on domain %s\n", genpd->name);
>> +
>> +	return ret;
>> +}
>> +
>> +static int scpsys_power_off(struct generic_pm_domain *genpd)
>> +{
>> +	struct scpsys_domain *pd = container_of(genpd, struct scpsys_domain, genpd);
>> +	struct scpsys *scpsys = pd->scpsys;
>> +	void __iomem *ctl_addr = scpsys->base + pd->data->ctl_offs;
>> +	int ret, tmp;
>> +	u32 val;
>> +
>> +	ret = scpsys_bus_protect_enable(pd);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = scpsys_sram_disable(pd, ctl_addr);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	/* subsys power off */
>> +	val = readl(ctl_addr);
>> +	val |= PWR_ISO_BIT;
>> +	writel(val, ctl_addr);
>> +
>> +	val &= ~PWR_RST_B_BIT;
>> +	writel(val, ctl_addr);
>> +
>> +	val |= PWR_CLK_DIS_BIT;
>> +	writel(val, ctl_addr);
>> +
>> +	val &= ~PWR_ON_BIT;
>> +	writel(val, ctl_addr);
>> +
>> +	val &= ~PWR_ON_2ND_BIT;
>> +	writel(val, ctl_addr);
>> +
>> +	/* wait until PWR_ACK = 0 */
>> +	ret = readx_poll_timeout(scpsys_domain_is_on, pd, tmp, tmp == 0, MTK_POLL_DELAY_US,
>> +				 MTK_POLL_TIMEOUT);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	clk_bulk_disable(pd->num_clks, pd->clks);
>> +
>> +	return 0;
>> +}
>> +
>> +static int scpsys_add_one_domain(struct scpsys *scpsys, struct device_node *node)
>> +{
>> +	const struct scpsys_domain_data *domain_data;
>> +	struct scpsys_domain *pd;
>> +	int i, ret;
>> +	u32 id;
>> +
>> +	ret = of_property_read_u32(node, "reg", &id);
>> +	if (ret) {
>> +		dev_err(scpsys->dev, "%pOFn: failed to retrieve domain id from reg: %d\n", node,
>> +			ret);
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (id >= scpsys->soc_data->num_domains) {
>> +		dev_err(scpsys->dev, "%pOFn: invalid domain id %d\n", node, id);
>> +		return -EINVAL;
>> +	}
>> +
>> +	domain_data = &scpsys->soc_data->domains[id];
>> +	if (!domain_data) {
>> +		dev_err(scpsys->dev, "%pOFn: undefined domain id %d\n", node, id);
>> +		return -EINVAL;
>> +	}
>> +
>> +	pd = devm_kzalloc(scpsys->dev, sizeof(*pd), GFP_KERNEL);
>> +	if (!pd)
>> +		return -ENOMEM;
>> +
>> +	pd->data = domain_data;
>> +	pd->scpsys = scpsys;
>> +
>> +	pd->infracfg = syscon_regmap_lookup_by_phandle(node, "mediatek,infracfg");
>> +	if (IS_ERR(pd->infracfg))
>> +		pd->infracfg = NULL;
>> +
>> +	pd->num_clks = of_clk_get_parent_count(node);
>> +	if (pd->num_clks > 0) {
>> +		pd->clks = devm_kcalloc(scpsys->dev, pd->num_clks, sizeof(*pd->clks), GFP_KERNEL);
>> +		if (!pd->clks)
>> +			return -ENOMEM;
>> +	} else {
>> +		pd->num_clks = 0;
>> +	}
>> +
>> +	for (i = 0; i < pd->num_clks; i++) {
>> +		pd->clks[i].clk = of_clk_get(node, i);
> 
> Is it possible to have a better way that we could use of_clk_bulk_get()?
> 
>> +		if (IS_ERR(pd->clks[i].clk)) {
>> +			ret = PTR_ERR(pd->clks[i].clk);
>> +			dev_err(scpsys->dev, "%pOFn: failed to get clk at index %d: %d\n", node, i,
>> +				ret);
>> +			return ret;
>> +		}
>> +	}
>> +
>> +	ret = clk_bulk_prepare(pd->num_clks, pd->clks);
>> +	if (ret)
>> +		goto err_put_clocks;
>> +
>> +	/*
>> +	 * Initially turn on all domains to make the domains usable
>> +	 * with !CONFIG_PM and to get the hardware in sync with the
>> +	 * software.  The unused domains will be switched off during
>> +	 * late_init time.
>> +	 */
>> +	ret = scpsys_power_on(&pd->genpd);
>> +	if (ret < 0) {
>> +		dev_err(scpsys->dev, "failed to power on domain %pOFN with error %d\n", node, ret);
>> +		goto err_unprepare_clocks;
>> +	}
>> +
>> +	pd->genpd.name = node->name;
>> +	pd->genpd.power_off = scpsys_power_off;
>> +	pd->genpd.power_on = scpsys_power_on;
>> +
>> +	pm_genpd_init(&pd->genpd, NULL, false);
>> +
>> +	scpsys->domains[id] = &pd->genpd;
>> +	return 0;
>> +
>> +err_unprepare_clocks:
>> +	clk_bulk_unprepare(pd->num_clks, pd->clks);
>> +err_put_clocks:
>> +	clk_bulk_put(pd->num_clks, pd->clks);
>> +	devm_kfree(scpsys->dev, pd->clks);
>> +	pd->num_clks = 0;
>> +	return ret;
>> +}
>> +
>> +static int scpsys_add_subdomain(struct scpsys *scpsys, struct device_node *parent)
>> +{
>> +	struct device_node *child;
>> +	struct generic_pm_domain *child_pd, *parent_pd;
>> +	int ret;
>> +
>> +	for_each_child_of_node(parent, child) {
>> +		u32 id;
>> +
>> +		ret = of_property_read_u32(parent, "reg", &id);
>> +		if (ret) {
>> +			dev_err(scpsys->dev, "%pOFn: failed to get parent domain id: %d\n", child,
>> +				ret);
>> +			goto err_put_node;
>> +		}
>> +		parent_pd = scpsys->pd_data.domains[id];
>> +
> 
> nit. Could we move parent outside(in front of) the loop?
> 

Yes, I think that makes sense.

Regards,
Matthias

Re: [PATCH 06/12] soc: mediatek: pm-domains: Add SMI block as bus protection block

[Weiyi Lu]

On Thu, 2020-09-10 at 19:28 +0200, Enric Balletbo i Serra wrote:
> From: Matthias Brugger <mbrugger@suse.com>
> 
> Apart from the infracfg block, the SMI block is used to enable the bus
> protection for some power domains. Add support for this block.
> 
> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> 
>  drivers/soc/mediatek/mtk-pm-domains.c | 64 ++++++++++++++++++++-------
>  include/linux/soc/mediatek/infracfg.h |  6 +++
>  2 files changed, 53 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
> index f609c2d454fa..3aa430a60602 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.c
> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> @@ -56,8 +56,25 @@
>  
>  #define SPM_MAX_BUS_PROT_DATA		3
>  
> +#define _BUS_PROT(_mask, _set, _clr, _sta, _update) {	\
> +		.bus_prot_mask = (_mask),		\
> +		.bus_prot_set = _set,			\
> +		.bus_prot_clr = _clr,			\
> +		.bus_prot_sta = _sta,			\
> +		.bus_prot_reg_update = _update,		\
> +	}
> +
> +#define BUS_PROT_WR(_mask, _set, _clr, _sta)		\
> +		_BUS_PROT(_mask, _set, _clr, _sta, false)
> +
> +#define BUS_PROT_UPDATE(_mask, _set, _clr, _sta)		\
> +		_BUS_PROT(_mask, _set, _clr, _sta, true)
> +
>  struct scpsys_bus_prot_data {
>  	u32 bus_prot_mask;
> +	u32 bus_prot_set;
> +	u32 bus_prot_clr;
> +	u32 bus_prot_sta;
>  	bool bus_prot_reg_update;
>  };
>  
> @@ -69,6 +86,7 @@ struct scpsys_bus_prot_data {
>   * @sram_pdn_ack_bits: The mask for sram power control acked bits.
>   * @caps: The flag for active wake-up action.
>   * @bp_infracfg: bus protection for infracfg subsystem
> + * @bp_smi: bus protection for smi subsystem
>   */
>  struct scpsys_domain_data {
>  	u32 sta_mask;
> @@ -77,6 +95,7 @@ struct scpsys_domain_data {
>  	u32 sram_pdn_ack_bits;
>  	u8 caps;
>  	const struct scpsys_bus_prot_data bp_infracfg[SPM_MAX_BUS_PROT_DATA];
> +	const struct scpsys_bus_prot_data bp_smi[SPM_MAX_BUS_PROT_DATA];
>  };
>  
>  struct scpsys_domain {
> @@ -86,6 +105,7 @@ struct scpsys_domain {
>  	int num_clks;
>  	struct clk_bulk_data *clks;
>  	struct regmap *infracfg;
> +	struct regmap *smi;
>  };
>  
>  struct scpsys_soc_data {
> @@ -173,9 +193,9 @@ static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, st
>  		if (bpd[i].bus_prot_reg_update)
>  			regmap_update_bits(regmap, INFRA_TOPAXI_PROTECTEN, mask, mask);
>  		else
> -			regmap_write(regmap, INFRA_TOPAXI_PROTECTEN_SET, mask);
> +			regmap_write(regmap, bpd[i].bus_prot_set, mask);
>  

Could it be?

if (bpd[i].bus_prot_set)
	regmap_write(regmap, bpd[i].bus_prot_set, mask);
else
	regmap_update_bits(regmap, INFRA_TOPAXI_PROTECTEN, mask, mask);

> -		ret = regmap_read_poll_timeout(regmap, INFRA_TOPAXI_PROTECTSTA1,
> +		ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
>  					       val, (val & mask) == mask,
>  					       MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
>  		if (ret)
> @@ -191,7 +211,11 @@ static int scpsys_bus_protect_enable(struct scpsys_domain *pd)
>  	int ret;
>  
>  	ret = _scpsys_bus_protect_enable(bpd, pd->infracfg);
> -	return ret;
> +	if (ret)
> +		return ret;
> +
> +	bpd = pd->data->bp_smi;
> +	return _scpsys_bus_protect_enable(bpd, pd->smi);
>  }
>  
>  static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
> @@ -206,11 +230,11 @@ static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
>  			return 0;
>  
>  		if (bpd[i].bus_prot_reg_update)

ditto.

> -			regmap_update_bits(regmap, INFRA_TOPAXI_PROTECTEN, mask, 0);
> +			regmap_update_bits(regmap, bpd[i].bus_prot_set, mask, 0);
>  		else
> -			regmap_write(regmap, INFRA_TOPAXI_PROTECTEN_CLR, mask);
> +			regmap_write(regmap, bpd[i].bus_prot_clr, mask);
>  
> -		ret = regmap_read_poll_timeout(regmap, INFRA_TOPAXI_PROTECTSTA1,
> +		ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
>  					       val, !(val & mask),
>  					       MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
>  		if (ret)
> @@ -226,7 +250,11 @@ static int scpsys_bus_protect_disable(struct scpsys_domain *pd)
>  	int ret;
>  
>  	ret = _scpsys_bus_protect_disable(bpd, pd->infracfg);
> -	return ret;
> +	if (ret)
> +		return ret;
> +
> +	bpd = pd->data->bp_smi;
> +	return _scpsys_bus_protect_disable(bpd, pd->smi);

It should have a reverse order compared to the enable control. But I'd
like to make it more flexible to any sequence, like INFRA->SMI->INFRA

>  }
>  
>  static int scpsys_power_on(struct generic_pm_domain *genpd)
> @@ -360,6 +388,10 @@ static int scpsys_add_one_domain(struct scpsys *scpsys, struct device_node *node
>  	if (IS_ERR(pd->infracfg))
>  		pd->infracfg = NULL;
>  
> +	pd->smi = syscon_regmap_lookup_by_phandle(node, "mediatek,smi");
> +	if (IS_ERR(pd->smi))
> +		pd->smi = NULL;
> +
>  	pd->num_clks = of_clk_get_parent_count(node);
>  	if (pd->num_clks > 0) {
>  		pd->clks = devm_kcalloc(scpsys->dev, pd->num_clks, sizeof(*pd->clks), GFP_KERNEL);
> @@ -532,10 +564,9 @@ static const struct scpsys_domain_data scpsys_domain_data_mt8173[] = {
>  		.ctl_offs = SPM_DIS_PWR_CON,
>  		.sram_pdn_bits = GENMASK(11, 8),
>  		.sram_pdn_ack_bits = GENMASK(12, 12),
> -		.bp_infracfg[0] = {
> -			.bus_prot_reg_update = true,
> -			.bus_prot_mask = MT8173_TOP_AXI_PROT_EN_MM_M0 |
> -				MT8173_TOP_AXI_PROT_EN_MM_M1,
> +		.bp_infracfg = {
> +			BUS_PROT_UPDATE_MT8173(MT8173_TOP_AXI_PROT_EN_MM_M0 |
> +					       MT8173_TOP_AXI_PROT_EN_MM_M1),
>  		},
>  	},
>  	[MT8173_POWER_DOMAIN_VENC_LT] = {
> @@ -574,12 +605,11 @@ static const struct scpsys_domain_data scpsys_domain_data_mt8173[] = {
>  		.ctl_offs = SPM_MFG_PWR_CON,
>  		.sram_pdn_bits = GENMASK(13, 8),
>  		.sram_pdn_ack_bits = GENMASK(21, 16),
> -		.bp_infracfg[0] = {
> -			.bus_prot_reg_update = true,
> -			.bus_prot_mask = MT8173_TOP_AXI_PROT_EN_MFG_S |
> -				MT8173_TOP_AXI_PROT_EN_MFG_M0 |
> -				MT8173_TOP_AXI_PROT_EN_MFG_M1 |
> -				MT8173_TOP_AXI_PROT_EN_MFG_SNOOP_OUT,
> +		.bp_infracfg = {
> +			BUS_PROT_UPDATE_MT8173(MT8173_TOP_AXI_PROT_EN_MFG_S |
> +					       MT8173_TOP_AXI_PROT_EN_MFG_M0 |
> +					       MT8173_TOP_AXI_PROT_EN_MFG_M1 |
> +					       MT8173_TOP_AXI_PROT_EN_MFG_SNOOP_OUT),
>  		},
>  	},
>  };
> diff --git a/include/linux/soc/mediatek/infracfg.h b/include/linux/soc/mediatek/infracfg.h
> index f967d02cc2ff..3f18cddffb44 100644
> --- a/include/linux/soc/mediatek/infracfg.h
> +++ b/include/linux/soc/mediatek/infracfg.h
> @@ -37,6 +37,12 @@
>  #define INFRA_TOPAXI_PROTECTEN_SET	0x0260
>  #define INFRA_TOPAXI_PROTECTEN_CLR	0x0264
>  
> +#define BUS_PROT_UPDATE_MT8173(_mask)			\
> +	BUS_PROT_UPDATE(_mask,				\
> +			INFRA_TOPAXI_PROTECTEN,		\
> +			INFRA_TOPAXI_PROTECTEN_CLR,	\
> +			INFRA_TOPAXI_PROTECTSTA1)
> +
>  int mtk_infracfg_set_bus_protection(struct regmap *infracfg, u32 mask,
>  		bool reg_update);
>  int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask,

Re: [PATCH 08/12] soc: mediatek: pm-domains: Add subsystem clocks

[Weiyi Lu]

On Thu, 2020-09-10 at 19:28 +0200, Enric Balletbo i Serra wrote:
> From: Matthias Brugger <mbrugger@suse.com>
> 
> For the bus protection operations, some subsystem clocks need to be enabled
> before releasing the protection. This patch identifies the subsystem clocks
> by it's name.
> 
> Suggested-by: Weiyi Lu <weiyi.lu@mediatek.com>
> [Adapted the patch to the mtk-pm-domains driver]
> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> 
>  drivers/soc/mediatek/mtk-pm-domains.c | 82 +++++++++++++++++++++++----
>  1 file changed, 70 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
> index 0802eccc3a0b..52a93a87e313 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.c
> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> @@ -3,6 +3,7 @@
>   * Copyright (c) 2020 Collabora Ltd.
>   */
>  #include <linux/clk.h>
> +#include <linux/clk-provider.h>
>  #include <linux/init.h>
>  #include <linux/io.h>
>  #include <linux/iopoll.h>
> @@ -81,6 +82,8 @@ struct scpsys_bus_prot_data {
>  	bool bus_prot_reg_update;
>  };
>  
> +#define MAX_SUBSYS_CLKS 10
> +
>  /**
>   * struct scpsys_domain_data - scp domain data for power on/off flow
>   * @sta_mask: The mask for power on/off status bit.
> @@ -107,6 +110,8 @@ struct scpsys_domain {
>  	struct scpsys *scpsys;
>  	int num_clks;
>  	struct clk_bulk_data *clks;
> +	int num_subsys_clks;
> +	struct clk_bulk_data *subsys_clks;
>  	struct regmap *infracfg;
>  	struct regmap *smi;
>  };
> @@ -309,16 +314,22 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
>  	val |= PWR_RST_B_BIT;
>  	writel(val, ctl_addr);
>  
> +	ret = clk_bulk_enable(pd->num_subsys_clks, pd->subsys_clks);
> +	if (ret)
> +		goto err_pwr_ack;
> +
>  	ret = scpsys_sram_enable(pd, ctl_addr);
>  	if (ret < 0)
> -		goto err_pwr_ack;
> +		goto err_sram;
>  
>  	ret = scpsys_bus_protect_disable(pd);
>  	if (ret < 0)
> -		goto err_pwr_ack;
> +		goto err_sram;
>  
>  	return 0;
>  
> +err_sram:
> +	clk_bulk_disable(pd->num_subsys_clks, pd->subsys_clks);
>  err_pwr_ack:
>  	clk_bulk_disable(pd->num_clks, pd->clks);
>  	dev_err(scpsys->dev, "Failed to power on domain %s\n", genpd->name);
> @@ -342,6 +353,8 @@ static int scpsys_power_off(struct generic_pm_domain *genpd)
>  	if (ret < 0)
>  		return ret;
>  
> +	clk_bulk_disable(pd->num_subsys_clks, pd->subsys_clks);
> +
>  	/* subsys power off */
>  	val = readl(ctl_addr);
>  	val |= PWR_ISO_BIT;
> @@ -374,8 +387,11 @@ static int scpsys_add_one_domain(struct scpsys *scpsys, struct device_node *node
>  {
>  	const struct scpsys_domain_data *domain_data;
>  	struct scpsys_domain *pd;
> -	int i, ret;
> +	int i, ret, num_clks;
>  	u32 id;
> +	int clk_ind = 0;
> +	struct property *prop;
> +	const char *clk_name;
>  
>  	ret = of_property_read_u32(node, "reg", &id);
>  	if (ret) {
> @@ -410,28 +426,60 @@ static int scpsys_add_one_domain(struct scpsys *scpsys, struct device_node *node
>  	if (IS_ERR(pd->smi))
>  		pd->smi = NULL;
>  
> -	pd->num_clks = of_clk_get_parent_count(node);
> -	if (pd->num_clks > 0) {
> +	num_clks = of_clk_get_parent_count(node);
> +	if (num_clks > 0) {
> +		/* Calculate number of subsys_clks */
> +		of_property_for_each_string(node, "clock-names", prop, clk_name) {
> +			char *subsys;
> +
> +			subsys = strchr(clk_name, '-');
> +			if (subsys)
> +				pd->num_subsys_clks++;
> +			else
> +				pd->num_clks++;
> +		}
> +

In fact, I don't like the clock naming rules, as Matthias mentioned
before. So in my work v17[1]
I put subsystem clocks under each power domain sub-node without giving
the clock name but put the basic clocks under the power controller node.

[1] https://patchwork.kernel.org/patch/11703191/


>  		pd->clks = devm_kcalloc(scpsys->dev, pd->num_clks, sizeof(*pd->clks), GFP_KERNEL);
>  		if (!pd->clks)
>  			return -ENOMEM;
> -	} else {
> -		pd->num_clks = 0;
> +
> +		pd->subsys_clks = devm_kcalloc(scpsys->dev, pd->num_subsys_clks,
> +					       sizeof(*pd->subsys_clks), GFP_KERNEL);
> +		if (!pd->subsys_clks)
> +			return -ENOMEM;
>  	}
>  
>  	for (i = 0; i < pd->num_clks; i++) {
> -		pd->clks[i].clk = of_clk_get(node, i);
> -		if (IS_ERR(pd->clks[i].clk)) {
> -			ret = PTR_ERR(pd->clks[i].clk);
> +		struct clk *clk = of_clk_get(node, i);
> +		if (IS_ERR(clk)) {
> +			ret = PTR_ERR(clk);
>  			dev_err(scpsys->dev, "%pOFn: failed to get clk at index %d: %d\n", node, i,
>  				ret);
> -			return ret;
> +			goto err_put_clocks;
> +		}
> +
> +		pd->clks[clk_ind++].clk = clk;
> +	}
> +
> +	for (i = 0; i < pd->num_subsys_clks; i++) {
> +		struct clk *clk = of_clk_get(node, i + clk_ind);
> +		if (IS_ERR(clk)) {
> +			ret = PTR_ERR(clk);
> +			dev_err(scpsys->dev, "%pOFn: failed to get clk at index %d: %d\n", node,
> +				i + clk_ind, ret);
> +			goto err_put_subsys_clocks;
>  		}
> +
> +		pd->subsys_clks[i].clk = clk;
>  	}
>  
> +	ret = clk_bulk_prepare(pd->num_subsys_clks, pd->subsys_clks);
> +	if (ret)
> +		goto err_put_subsys_clocks;
> +
>  	ret = clk_bulk_prepare(pd->num_clks, pd->clks);
>  	if (ret)
> -		goto err_put_clocks;
> +		goto err_unprepare_subsys_clocks;
>  
>  	/*
>  	 * Initially turn on all domains to make the domains usable
> @@ -456,6 +504,12 @@ static int scpsys_add_one_domain(struct scpsys *scpsys, struct device_node *node
>  
>  err_unprepare_clocks:
>  	clk_bulk_unprepare(pd->num_clks, pd->clks);
> +err_unprepare_subsys_clocks:
> +	clk_bulk_unprepare(pd->num_subsys_clks, pd->subsys_clks);
> +err_put_subsys_clocks:
> +	clk_bulk_put(pd->num_subsys_clks, pd->subsys_clks);
> +	devm_kfree(scpsys->dev, pd->subsys_clks);
> +	pd->num_subsys_clks = 0;
>  err_put_clocks:
>  	clk_bulk_put(pd->num_clks, pd->clks);
>  	devm_kfree(scpsys->dev, pd->clks);
> @@ -537,6 +591,10 @@ static void scpsys_remove_one_domain(struct scpsys_domain *pd)
>  	clk_bulk_unprepare(pd->num_clks, pd->clks);
>  	clk_bulk_put(pd->num_clks, pd->clks);
>  	pd->num_clks = 0;
> +
> +	clk_bulk_unprepare(pd->num_subsys_clks, pd->subsys_clks);
> +	clk_bulk_put(pd->num_subsys_clks, pd->subsys_clks);
> +	pd->num_subsys_clks = 0;
>  }
>  
>  static void scpsys_domain_cleanup(struct scpsys *scpsys)

Re: [PATCH 06/12] soc: mediatek: pm-domains: Add SMI block as bus protection block

[Matthias Brugger]

On 25/09/2020 12:45, Weiyi Lu wrote:
> On Thu, 2020-09-10 at 19:28 +0200, Enric Balletbo i Serra wrote:
>> From: Matthias Brugger <mbrugger@suse.com>
>>
>> Apart from the infracfg block, the SMI block is used to enable the bus
>> protection for some power domains. Add support for this block.
>>
>> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> ---
>>
>>   drivers/soc/mediatek/mtk-pm-domains.c | 64 ++++++++++++++++++++-------
>>   include/linux/soc/mediatek/infracfg.h |  6 +++
>>   2 files changed, 53 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
>> index f609c2d454fa..3aa430a60602 100644
>> --- a/drivers/soc/mediatek/mtk-pm-domains.c
>> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
>> @@ -56,8 +56,25 @@
>>   
>>   #define SPM_MAX_BUS_PROT_DATA		3
>>   
>> +#define _BUS_PROT(_mask, _set, _clr, _sta, _update) {	\
>> +		.bus_prot_mask = (_mask),		\
>> +		.bus_prot_set = _set,			\
>> +		.bus_prot_clr = _clr,			\
>> +		.bus_prot_sta = _sta,			\
>> +		.bus_prot_reg_update = _update,		\
>> +	}
>> +
>> +#define BUS_PROT_WR(_mask, _set, _clr, _sta)		\
>> +		_BUS_PROT(_mask, _set, _clr, _sta, false)
>> +
>> +#define BUS_PROT_UPDATE(_mask, _set, _clr, _sta)		\
>> +		_BUS_PROT(_mask, _set, _clr, _sta, true)
>> +
>>   struct scpsys_bus_prot_data {
>>   	u32 bus_prot_mask;
>> +	u32 bus_prot_set;
>> +	u32 bus_prot_clr;
>> +	u32 bus_prot_sta;
>>   	bool bus_prot_reg_update;
>>   };
>>   
>> @@ -69,6 +86,7 @@ struct scpsys_bus_prot_data {
>>    * @sram_pdn_ack_bits: The mask for sram power control acked bits.
>>    * @caps: The flag for active wake-up action.
>>    * @bp_infracfg: bus protection for infracfg subsystem
>> + * @bp_smi: bus protection for smi subsystem
>>    */
>>   struct scpsys_domain_data {
>>   	u32 sta_mask;
>> @@ -77,6 +95,7 @@ struct scpsys_domain_data {
>>   	u32 sram_pdn_ack_bits;
>>   	u8 caps;
>>   	const struct scpsys_bus_prot_data bp_infracfg[SPM_MAX_BUS_PROT_DATA];
>> +	const struct scpsys_bus_prot_data bp_smi[SPM_MAX_BUS_PROT_DATA];
>>   };
>>   
>>   struct scpsys_domain {
>> @@ -86,6 +105,7 @@ struct scpsys_domain {
>>   	int num_clks;
>>   	struct clk_bulk_data *clks;
>>   	struct regmap *infracfg;
>> +	struct regmap *smi;
>>   };
>>   
>>   struct scpsys_soc_data {
>> @@ -173,9 +193,9 @@ static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, st
>>   		if (bpd[i].bus_prot_reg_update)
>>   			regmap_update_bits(regmap, INFRA_TOPAXI_PROTECTEN, mask, mask);
>>   		else
>> -			regmap_write(regmap, INFRA_TOPAXI_PROTECTEN_SET, mask);
>> +			regmap_write(regmap, bpd[i].bus_prot_set, mask);
>>   
> 
> Could it be?
> 
> if (bpd[i].bus_prot_set)
> 	regmap_write(regmap, bpd[i].bus_prot_set, mask);
> else
> 	regmap_update_bits(regmap, INFRA_TOPAXI_PROTECTEN, mask, mask);
> 

Nice catch. That's a bug here:
regmap_update_bits(regmap, INFRA_TOPAXI_PROTECTEN, mask, mask)
should be
regmap_update_bits(regmap, bpd[i].bus_prot_set, mask, mask)

Regarding your question. No we can't do that as we use bus_prot_set variable for 
write and update and decide on bus_prot_reg_update which method to use.

>> -		ret = regmap_read_poll_timeout(regmap, INFRA_TOPAXI_PROTECTSTA1,
>> +		ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
>>   					       val, (val & mask) == mask,
>>   					       MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
>>   		if (ret)
>> @@ -191,7 +211,11 @@ static int scpsys_bus_protect_enable(struct scpsys_domain *pd)
>>   	int ret;
>>   
>>   	ret = _scpsys_bus_protect_enable(bpd, pd->infracfg);
>> -	return ret;
>> +	if (ret)
>> +		return ret;
>> +
>> +	bpd = pd->data->bp_smi;
>> +	return _scpsys_bus_protect_enable(bpd, pd->smi);
>>   }
>>   
>>   static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
>> @@ -206,11 +230,11 @@ static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
>>   			return 0;
>>   
>>   		if (bpd[i].bus_prot_reg_update)
> 
> ditto.
> 
>> -			regmap_update_bits(regmap, INFRA_TOPAXI_PROTECTEN, mask, 0);
>> +			regmap_update_bits(regmap, bpd[i].bus_prot_set, mask, 0);

As you can see here we use bpd[i].bus_prot_set
You could argue that we could check the bus_prot_clr to decide if we need an 
update or not, but I prefer an explicit variable to flag this. Implicite 
reasoning depending on values for bus_prot_clr will make the code harder to read 
and won't save a lot of memory.

>>   		else
>> -			regmap_write(regmap, INFRA_TOPAXI_PROTECTEN_CLR, mask);
>> +			regmap_write(regmap, bpd[i].bus_prot_clr, mask);
>>   
>> -		ret = regmap_read_poll_timeout(regmap, INFRA_TOPAXI_PROTECTSTA1,
>> +		ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
>>   					       val, !(val & mask),
>>   					       MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
>>   		if (ret)
>> @@ -226,7 +250,11 @@ static int scpsys_bus_protect_disable(struct scpsys_domain *pd)
>>   	int ret;
>>   
>>   	ret = _scpsys_bus_protect_disable(bpd, pd->infracfg);
>> -	return ret;
>> +	if (ret)
>> +		return ret;
>> +
>> +	bpd = pd->data->bp_smi;
>> +	return _scpsys_bus_protect_disable(bpd, pd->smi);
> 
> It should have a reverse order compared to the enable control. But I'd
> like to make it more flexible to any sequence, like INFRA->SMI->INFRA

Correct, we whould disable first SMI and then infracfg. Good catch!

Right now we don't have any hardware supported that does 
infracfg->SMI->infracfg. When that comes up we will need a good solution for the 
driver :)

Thanks for your review!
Matthias

> 
>>   }
>>   
>>   static int scpsys_power_on(struct generic_pm_domain *genpd)
>> @@ -360,6 +388,10 @@ static int scpsys_add_one_domain(struct scpsys *scpsys, struct device_node *node
>>   	if (IS_ERR(pd->infracfg))
>>   		pd->infracfg = NULL;
>>   
>> +	pd->smi = syscon_regmap_lookup_by_phandle(node, "mediatek,smi");
>> +	if (IS_ERR(pd->smi))
>> +		pd->smi = NULL;
>> +
>>   	pd->num_clks = of_clk_get_parent_count(node);
>>   	if (pd->num_clks > 0) {
>>   		pd->clks = devm_kcalloc(scpsys->dev, pd->num_clks, sizeof(*pd->clks), GFP_KERNEL);
>> @@ -532,10 +564,9 @@ static const struct scpsys_domain_data scpsys_domain_data_mt8173[] = {
>>   		.ctl_offs = SPM_DIS_PWR_CON,
>>   		.sram_pdn_bits = GENMASK(11, 8),
>>   		.sram_pdn_ack_bits = GENMASK(12, 12),
>> -		.bp_infracfg[0] = {
>> -			.bus_prot_reg_update = true,
>> -			.bus_prot_mask = MT8173_TOP_AXI_PROT_EN_MM_M0 |
>> -				MT8173_TOP_AXI_PROT_EN_MM_M1,
>> +		.bp_infracfg = {
>> +			BUS_PROT_UPDATE_MT8173(MT8173_TOP_AXI_PROT_EN_MM_M0 |
>> +					       MT8173_TOP_AXI_PROT_EN_MM_M1),
>>   		},
>>   	},
>>   	[MT8173_POWER_DOMAIN_VENC_LT] = {
>> @@ -574,12 +605,11 @@ static const struct scpsys_domain_data scpsys_domain_data_mt8173[] = {
>>   		.ctl_offs = SPM_MFG_PWR_CON,
>>   		.sram_pdn_bits = GENMASK(13, 8),
>>   		.sram_pdn_ack_bits = GENMASK(21, 16),
>> -		.bp_infracfg[0] = {
>> -			.bus_prot_reg_update = true,
>> -			.bus_prot_mask = MT8173_TOP_AXI_PROT_EN_MFG_S |
>> -				MT8173_TOP_AXI_PROT_EN_MFG_M0 |
>> -				MT8173_TOP_AXI_PROT_EN_MFG_M1 |
>> -				MT8173_TOP_AXI_PROT_EN_MFG_SNOOP_OUT,
>> +		.bp_infracfg = {
>> +			BUS_PROT_UPDATE_MT8173(MT8173_TOP_AXI_PROT_EN_MFG_S |
>> +					       MT8173_TOP_AXI_PROT_EN_MFG_M0 |
>> +					       MT8173_TOP_AXI_PROT_EN_MFG_M1 |
>> +					       MT8173_TOP_AXI_PROT_EN_MFG_SNOOP_OUT),
>>   		},
>>   	},
>>   };
>> diff --git a/include/linux/soc/mediatek/infracfg.h b/include/linux/soc/mediatek/infracfg.h
>> index f967d02cc2ff..3f18cddffb44 100644
>> --- a/include/linux/soc/mediatek/infracfg.h
>> +++ b/include/linux/soc/mediatek/infracfg.h
>> @@ -37,6 +37,12 @@
>>   #define INFRA_TOPAXI_PROTECTEN_SET	0x0260
>>   #define INFRA_TOPAXI_PROTECTEN_CLR	0x0264
>>   
>> +#define BUS_PROT_UPDATE_MT8173(_mask)			\
>> +	BUS_PROT_UPDATE(_mask,				\
>> +			INFRA_TOPAXI_PROTECTEN,		\
>> +			INFRA_TOPAXI_PROTECTEN_CLR,	\
>> +			INFRA_TOPAXI_PROTECTSTA1)
>> +
>>   int mtk_infracfg_set_bus_protection(struct regmap *infracfg, u32 mask,
>>   		bool reg_update);
>>   int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask,
> 

Re: [PATCH 08/12] soc: mediatek: pm-domains: Add subsystem clocks

[Matthias Brugger]

On 25/09/2020 12:55, Weiyi Lu wrote:
> On Thu, 2020-09-10 at 19:28 +0200, Enric Balletbo i Serra wrote:
>> From: Matthias Brugger <mbrugger@suse.com>
>>
>> For the bus protection operations, some subsystem clocks need to be enabled
>> before releasing the protection. This patch identifies the subsystem clocks
>> by it's name.
>>
>> Suggested-by: Weiyi Lu <weiyi.lu@mediatek.com>
>> [Adapted the patch to the mtk-pm-domains driver]
>> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> ---
>>
>>   drivers/soc/mediatek/mtk-pm-domains.c | 82 +++++++++++++++++++++++----
>>   1 file changed, 70 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
>> index 0802eccc3a0b..52a93a87e313 100644
>> --- a/drivers/soc/mediatek/mtk-pm-domains.c
>> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
[...]
>>   
>> -	pd->num_clks = of_clk_get_parent_count(node);
>> -	if (pd->num_clks > 0) {
>> +	num_clks = of_clk_get_parent_count(node);
>> +	if (num_clks > 0) {
>> +		/* Calculate number of subsys_clks */
>> +		of_property_for_each_string(node, "clock-names", prop, clk_name) {
>> +			char *subsys;
>> +
>> +			subsys = strchr(clk_name, '-');
>> +			if (subsys)
>> +				pd->num_subsys_clks++;
>> +			else
>> +				pd->num_clks++;
>> +		}
>> +
> 
> In fact, I don't like the clock naming rules, as Matthias mentioned
> before. So in my work v17[1]
> I put subsystem clocks under each power domain sub-node without giving
> the clock name but put the basic clocks under the power controller node.
> 
> [1] https://patchwork.kernel.org/patch/11703191/

The quick answer, there is no perfect solution to our problem. If we put all 
basic clocks under the parent node, then we will have to have a list of basic 
clocks in the driver data for each SoC. Apart from that the DTS would not 
reflect the exact hardware, as the basic clocks needed for every power domain 
would be hidden in the driver data.

Given this, I think the best way is to distinguish the clocks by it's name, as 
done by you in earlier series of the scpsys. It will give us a cleaner device 
tree and we won't need to add long clock lists in the driver data. If I remember 
correctly Rob acknowledged your binding change for that:
https://patchwork.kernel.org/patch/11562521/

Regards,
Matthias

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
> 

Re: [PATCH 07/12] soc: mediatek: pm-domains: Add extra sram control

[Enric Balletbo i Serra]

Hi Matthias,

On 10/9/20 20:27, Matthias Brugger wrote:
> 
> 
> On 10/09/2020 19:28, Enric Balletbo i Serra wrote:
>> From: Matthias Brugger <mbrugger@suse.com>
>>
>> For some power domains like vpu_core on MT8183 whose sram need to do clock
>> and internal isolation while power on/off sram. We add a cap
>> "MTK_SCPD_SRAM_ISO" to judge if we need to do the extra sram isolation
>> control or not.
>>
>> Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
>> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> ---
>>
>>   drivers/soc/mediatek/mtk-pm-domains.c | 22 ++++++++++++++++++++--
>>   1 file changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c
>> b/drivers/soc/mediatek/mtk-pm-domains.c
>> index 3aa430a60602..0802eccc3a0b 100644
>> --- a/drivers/soc/mediatek/mtk-pm-domains.c
>> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
>> @@ -21,6 +21,7 @@
>>     #define MTK_SCPD_ACTIVE_WAKEUP        BIT(0)
>>   #define MTK_SCPD_FWAIT_SRAM        BIT(1)
>> +#define MTK_SCPD_SRAM_ISO        BIT(2)
>>   #define MTK_SCPD_CAPS(_scpd, _x)    ((_scpd)->data->caps & (_x))
>>     #define SPM_VDE_PWR_CON            0x0210
>> @@ -42,6 +43,8 @@
>>   #define PWR_ON_BIT            BIT(2)
>>   #define PWR_ON_2ND_BIT            BIT(3)
>>   #define PWR_CLK_DIS_BIT            BIT(4)
>> +#define PWR_SRAM_CLKISO_BIT        BIT(5)
>> +#define PWR_SRAM_ISOINT_B_BIT        BIT(6)
>>     #define PWR_STATUS_DISP            BIT(3)
>>   #define PWR_STATUS_MFG            BIT(4)
>> @@ -162,6 +165,14 @@ static int scpsys_sram_enable(struct scpsys_domain *pd,
>> void __iomem *ctl_addr)
>>       if (ret < 0)
>>           return ret;
>>   +    if (MTK_SCPD_CAPS(pd, MTK_SCPD_SRAM_ISO))    {
>> +        val = readl(ctl_addr) | PWR_SRAM_ISOINT_B_BIT;
>> +        writel(val, ctl_addr);
>> +        udelay(1);
>> +        val &= ~PWR_SRAM_CLKISO_BIT;
>> +        writel(val, ctl_addr);
>> +    }
>> +
>>       return 0;
>>   }
>>   @@ -171,8 +182,15 @@ static int scpsys_sram_disable(struct scpsys_domain
>> *pd, void __iomem *ctl_addr)
>>       u32 val;
>>       int tmp;
>>   -    val = readl(ctl_addr);
>> -    val |= pd->data->sram_pdn_bits;
>> +    if (MTK_SCPD_CAPS(pd, MTK_SCPD_SRAM_ISO))    {
>> +        val = readl(ctl_addr) | PWR_SRAM_CLKISO_BIT;
>> +        writel(val, ctl_addr);
>> +        val &= ~PWR_SRAM_ISOINT_B_BIT;
>> +        writel(val, ctl_addr);
>> +        udelay(1);
>> +    }
>> +
>> +    val = readl(ctl_addr) | pd->data->sram_pdn_bits;
> 
> Nit, I'd prefer:
> val = readl(ctl_addr);
> val |= pd->data->sram_pdn_bits;
> 

done in next version.

> 
>>       writel(val, ctl_addr);
>>         /* Either wait until SRAM_PDN_ACK all 1 or 0 */
>>