linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/12] soc: mediatek: pm-domains: Add new driver for SCPSYS power domains controller
@ 2020-10-01 16:01 Enric Balletbo i Serra
  2020-10-01 16:01 ` [PATCH v2 01/12] dt-bindings: power: Add bindings for the Mediatek " Enric Balletbo i Serra
                   ` (11 more replies)
  0 siblings, 12 replies; 30+ messages in thread
From: Enric Balletbo i Serra @ 2020-10-01 16:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Collabora Kernel ML, fparent, matthias.bgg, drinkcat, hsinyi,
	weiyi.lu, Rob Herring, devicetree, linux-arm-kernel,
	linux-mediatek

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.

Note that the MT8183 support is not ready to land yet because has some
dependencies, i.e. mmsys support is still missing. So, only patches from
1 to 9 are ready, the others are provided for reference and test.

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      |  184 +++
 arch/arm64/boot/dts/mediatek/mt8173.dtsi      |  164 ++-
 arch/arm64/boot/dts/mediatek/mt8183.dtsi      |  162 +++
 drivers/soc/mediatek/Kconfig                  |   13 +
 drivers/soc/mediatek/Makefile                 |    1 +
 drivers/soc/mediatek/mtk-infracfg.c           |    5 -
 drivers/soc/mediatek/mtk-mmsys.c              |    4 -
 drivers/soc/mediatek/mtk-pm-domains.c         | 1019 +++++++++++++++++
 include/dt-bindings/power/mt8183-power.h      |   26 +
 include/linux/soc/mediatek/infracfg.h         |   57 +
 10 files changed, 1577 insertions(+), 58 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


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

* [PATCH v2 01/12] dt-bindings: power: Add bindings for the Mediatek SCPSYS power domains controller
  2020-10-01 16:01 [PATCH v2 00/12] soc: mediatek: pm-domains: Add new driver for SCPSYS power domains controller Enric Balletbo i Serra
@ 2020-10-01 16:01 ` Enric Balletbo i Serra
  2020-10-01 16:01 ` [PATCH v2 02/12] soc: mediatek: Add MediaTek SCPSYS power domains Enric Balletbo i Serra
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 30+ messages in thread
From: Enric Balletbo i Serra @ 2020-10-01 16:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Collabora Kernel ML, fparent, matthias.bgg, drinkcat, hsinyi,
	weiyi.lu, Matthias Brugger, Rob Herring, devicetree,
	linux-arm-kernel, linux-mediatek

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

Changes in v2:
- Use generic node names (power-domain).
- Define valid values for common properties like #power-domain-cells.

 .../power/mediatek,power-controller.yaml      | 182 ++++++++++++++++++
 1 file changed, 182 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..9ac7a7694d61
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
@@ -0,0 +1,182 @@
+# 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:
+    const: power-controller
+
+  compatible:
+    enum:
+      - mediatek,mt8173-power-controller
+
+  '#power-domain-cells':
+    const: 1
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+patternProperties:
+  "^power-domain@[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:
+
+      '#power-domain-cells':
+        description:
+            Must be 0 for nodes representing a single PM domain and 1 for nodes
+            providing multiple PM domains.
+
+      '#address-cells':
+        const: 1
+
+      '#size-cells':
+        const: 0
+
+      reg:
+        description: |
+          Power domain index. Valid values are defined in:
+              "include/dt-bindings/power/mt8173-power.h" - for MT8173 type power domain.
+        maxItems: 1
+
+      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'
+
+required:
+  - compatible
+
+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 = "syscon", "simple-mfd";
+            reg = <0 0x10006000 0 0x1000>;
+
+            spm: power-controller {
+                compatible = "mediatek,mt8173-power-controller";
+                #address-cells = <1>;
+                #size-cells = <0>;
+                #power-domain-cells = <1>;
+
+                /* power domains of the SoC */
+                power-domain@MT8173_POWER_DOMAIN_VDEC {
+                    reg = <MT8173_POWER_DOMAIN_VDEC>;
+                    clocks = <&topckgen CLK_TOP_MM_SEL>;
+                    clock-names = "mm";
+                    #power-domain-cells = <0>;
+                };
+                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>;
+                };
+                power-domain@MT8173_POWER_DOMAIN_ISP {
+                    reg = <MT8173_POWER_DOMAIN_ISP>;
+                    clocks = <&topckgen CLK_TOP_MM_SEL>;
+                    clock-names = "mm";
+                    #power-domain-cells = <0>;
+                };
+                power-domain@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>;
+                };
+                power-domain@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>;
+                };
+                power-domain@MT8173_POWER_DOMAIN_AUDIO {
+                    reg = <MT8173_POWER_DOMAIN_AUDIO>;
+                    #power-domain-cells = <0>;
+                };
+                power-domain@MT8173_POWER_DOMAIN_USB {
+                    reg = <MT8173_POWER_DOMAIN_USB>;
+                    #power-domain-cells = <0>;
+                };
+                power-domain@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>;
+
+                    power-domain@MT8173_POWER_DOMAIN_MFG_2D {
+                        reg = <MT8173_POWER_DOMAIN_MFG_2D>;
+                        #address-cells = <1>;
+                        #size-cells = <0>;
+                        #power-domain-cells = <1>;
+
+                        power-domain@MT8173_POWER_DOMAIN_MFG {
+                            reg = <MT8173_POWER_DOMAIN_MFG>;
+                            #power-domain-cells = <0>;
+                            mediatek,infracfg = <&infracfg>;
+                        };
+                    };
+                };
+            };
+        };
+    };
-- 
2.28.0


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

* [PATCH v2 02/12] soc: mediatek: Add MediaTek SCPSYS power domains
  2020-10-01 16:01 [PATCH v2 00/12] soc: mediatek: pm-domains: Add new driver for SCPSYS power domains controller Enric Balletbo i Serra
  2020-10-01 16:01 ` [PATCH v2 01/12] dt-bindings: power: Add bindings for the Mediatek " Enric Balletbo i Serra
@ 2020-10-01 16:01 ` Enric Balletbo i Serra
  2020-10-05  1:39   ` Nicolas Boichat
  2020-10-01 16:01 ` [PATCH v2 03/12] arm64: dts: mediatek: Add mt8173 power domain controller Enric Balletbo i Serra
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 30+ messages in thread
From: Enric Balletbo i Serra @ 2020-10-01 16:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Collabora Kernel ML, fparent, matthias.bgg, drinkcat, hsinyi,
	weiyi.lu, Matthias Brugger, linux-arm-kernel, linux-mediatek

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

Changes in v2:
- Get base address from parent syscon. We have now a scpsys syscon node
  and a child for the SPM (System Power Manager).
- Use regmap API to acces de base address.

 drivers/soc/mediatek/Kconfig          |  13 +
 drivers/soc/mediatek/Makefile         |   1 +
 drivers/soc/mediatek/mtk-pm-domains.c | 628 ++++++++++++++++++++++++++
 3 files changed, 642 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..68886bf437f9
--- /dev/null
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -0,0 +1,628 @@
+// 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/regmap.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;
+	struct regmap *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, status2;
+
+	regmap_read(scpsys->base, scpsys->soc_data->pwr_sta_offs, &status);
+	status &= pd->data->sta_mask;
+
+	regmap_read(scpsys->base, scpsys->soc_data->pwr_sta2nd_offs, &status2);
+	status2 &= 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)
+{
+	u32 pdn_ack = pd->data->sram_pdn_ack_bits;
+	struct scpsys *scpsys = pd->scpsys;
+	u32 val;
+	int tmp;
+
+	regmap_read(scpsys->base, pd->data->ctl_offs, &val);
+	val &= ~pd->data->sram_pdn_bits;
+	regmap_write(scpsys->base, pd->data->ctl_offs, val);
+
+	/* Either wait until SRAM_PDN_ACK all 1 or 0 */
+	return regmap_read_poll_timeout(scpsys->base, pd->data->ctl_offs, tmp,
+					(tmp & pdn_ack) == 0, MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
+}
+
+static int scpsys_sram_disable(struct scpsys_domain *pd)
+{
+	u32 pdn_ack = pd->data->sram_pdn_ack_bits;
+	struct scpsys *scpsys = pd->scpsys;
+	u32 val;
+	int tmp;
+
+	regmap_read(scpsys->base, pd->data->ctl_offs, &val);
+	val |= pd->data->sram_pdn_bits;
+	regmap_write(scpsys->base, pd->data->ctl_offs, val);
+
+	/* Either wait until SRAM_PDN_ACK all 1 or 0 */
+	return regmap_read_poll_timeout(scpsys->base, pd->data->ctl_offs, 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;
+	int ret, tmp;
+	u32 val;
+
+	ret = clk_bulk_enable(pd->num_clks, pd->clks);
+	if (ret)
+		return ret;
+
+	/* subsys power on */
+	regmap_read(scpsys->base, pd->data->ctl_offs, &val);
+	val |= PWR_ON_BIT;
+	regmap_write(scpsys->base, pd->data->ctl_offs, val);
+	val |= PWR_ON_2ND_BIT;
+	regmap_write(scpsys->base, pd->data->ctl_offs, val);
+
+	/* 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;
+	regmap_write(scpsys->base, pd->data->ctl_offs, val);
+
+	val &= ~PWR_ISO_BIT;
+	regmap_write(scpsys->base, pd->data->ctl_offs, val);
+
+	val |= PWR_RST_B_BIT;
+	regmap_write(scpsys->base, pd->data->ctl_offs, val);
+
+	ret = scpsys_sram_enable(pd);
+	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);
+	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;
+	int ret, tmp;
+	u32 val;
+
+	ret = scpsys_bus_protect_enable(pd);
+	if (ret < 0)
+		return ret;
+
+	ret = scpsys_sram_disable(pd);
+	if (ret < 0)
+		return ret;
+
+	/* subsys power off */
+	regmap_read(scpsys->base, pd->data->ctl_offs, &val);
+	val |= PWR_ISO_BIT;
+	regmap_write(scpsys->base, pd->data->ctl_offs, val);
+
+	val &= ~PWR_RST_B_BIT;
+	regmap_write(scpsys->base, pd->data->ctl_offs, val);
+
+	val |= PWR_CLK_DIS_BIT;
+	regmap_write(scpsys->base, pd->data->ctl_offs, val);
+
+	val &= ~PWR_ON_BIT;
+	regmap_write(scpsys->base, pd->data->ctl_offs, val);
+
+	val &= ~PWR_ON_2ND_BIT;
+	regmap_write(scpsys->base, pd->data->ctl_offs, val);
+
+	/* 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_probe(scpsys->dev, ret,
+			      "%pOFn: failed to retrieve domain id from reg\n", node);
+		return -EINVAL;
+	}
+
+	if (id >= scpsys->soc_data->num_domains) {
+		dev_err_probe(scpsys->dev, -EINVAL, "%pOFn: invalid domain id %d\n", node, id);
+		return -EINVAL;
+	}
+
+	domain_data = &scpsys->soc_data->domains[id];
+	if (!domain_data) {
+		dev_err_probe(scpsys->dev, -EINVAL, "%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_probe(scpsys->dev, ret, "%pOFn: failed to get clk at index %d\n",
+				      node, i);
+			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_probe(scpsys->dev, ret, "failed to power on domain %pOFN\n", node);
+		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_probe(scpsys->dev, ret,
+				      "%pOFn: failed to get parent domain id\n", child);
+			goto err_put_node;
+		}
+		parent_pd = scpsys->pd_data.domains[id];
+
+		ret = scpsys_add_one_domain(scpsys, child);
+		if (ret)
+			goto err_put_node;
+
+		ret = of_property_read_u32(child, "reg", &id);
+		if (ret) {
+			dev_err_probe(scpsys->dev, ret,
+				      "%pOFn: failed to get child domain id\n", child);
+			goto err_put_node;
+		}
+		child_pd = scpsys->pd_data.domains[id];
+
+		ret = pm_genpd_add_subdomain(parent_pd, child_pd);
+		if (ret) {
+			dev_err_probe(scpsys->dev, ret,
+				      "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 device *parent;
+	struct scpsys *scpsys;
+	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;
+
+	parent = dev->parent;
+	if (!parent) {
+		dev_err(dev, "no parent for syscon devices\n");
+		return -ENODEV;
+	}
+
+	scpsys->base = syscon_node_to_regmap(parent->of_node);
+	if (IS_ERR(scpsys->base)) {
+		dev_err(dev, "no regmap available\n");
+		return PTR_ERR(scpsys->base);
+	}
+
+	ret = -ENODEV;
+	for_each_available_child_of_node(np, node) {
+		ret = scpsys_add_one_domain(scpsys, node);
+		if (ret) {
+			of_node_put(node);
+			goto err_cleanup_domains;
+		}
+
+		ret = scpsys_add_subdomain(scpsys, node);
+		if (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


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

* [PATCH v2 03/12] arm64: dts: mediatek: Add mt8173 power domain controller
  2020-10-01 16:01 [PATCH v2 00/12] soc: mediatek: pm-domains: Add new driver for SCPSYS power domains controller Enric Balletbo i Serra
  2020-10-01 16:01 ` [PATCH v2 01/12] dt-bindings: power: Add bindings for the Mediatek " Enric Balletbo i Serra
  2020-10-01 16:01 ` [PATCH v2 02/12] soc: mediatek: Add MediaTek SCPSYS power domains Enric Balletbo i Serra
@ 2020-10-01 16:01 ` Enric Balletbo i Serra
  2020-10-01 16:01 ` [PATCH v2 04/12] soc: mediatek: pm-domains: Add bus protection protocol Enric Balletbo i Serra
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 30+ messages in thread
From: Enric Balletbo i Serra @ 2020-10-01 16:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Collabora Kernel ML, fparent, matthias.bgg, drinkcat, hsinyi,
	weiyi.lu, Rob Herring, devicetree, linux-arm-kernel,
	linux-mediatek

Add power domain controller node for SoC mt8173.

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

Changes in v2:
- Add a scpsys syscon node as parent and a SPM (System Power Manager) as
  a child.

 arch/arm64/boot/dts/mediatek/mt8173.dtsi | 164 ++++++++++++++++-------
 1 file changed, 115 insertions(+), 49 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index 5e046f9d48ce..7fa870e4386a 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -450,16 +450,82 @@ pins1 {
 			};
 		};
 
-		scpsys: power-controller@10006000 {
-			compatible = "mediatek,mt8173-scpsys";
-			#power-domain-cells = <1>;
+		scpsys: syscon@10006000 {
+			compatible = "syscon", "simple-mfd";
 			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>;
+			#power-domain-cells = <1>;
+
+			/* System Power Manager */
+			spm: power-controller {
+				compatible = "mediatek,mt8173-power-controller";
+				#address-cells = <1>;
+				#size-cells = <0>;
+				#power-domain-cells = <1>;
+
+				/* power domains of the SoC */
+				power-domain@MT8173_POWER_DOMAIN_VDEC {
+					reg = <MT8173_POWER_DOMAIN_VDEC>;
+					clocks = <&topckgen CLK_TOP_MM_SEL>;
+					clock-names = "mm";
+					#power-domain-cells = <0>;
+				};
+				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>;
+				};
+				power-domain@MT8173_POWER_DOMAIN_ISP {
+					reg = <MT8173_POWER_DOMAIN_ISP>;
+					clocks = <&topckgen CLK_TOP_MM_SEL>;
+					clock-names = "mm";
+					#power-domain-cells = <0>;
+				};
+				power-domain@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>;
+				};
+				power-domain@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>;
+				};
+				power-domain@MT8173_POWER_DOMAIN_AUDIO {
+					reg = <MT8173_POWER_DOMAIN_AUDIO>;
+					#power-domain-cells = <0>;
+				};
+				power-domain@MT8173_POWER_DOMAIN_USB {
+					reg = <MT8173_POWER_DOMAIN_USB>;
+					#power-domain-cells = <0>;
+				};
+				power-domain@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>;
+
+					power-domain@MT8173_POWER_DOMAIN_MFG_2D {
+						reg = <MT8173_POWER_DOMAIN_MFG_2D>;
+						#address-cells = <1>;
+						#size-cells = <0>;
+						#power-domain-cells = <1>;
+
+						power-domain@MT8173_POWER_DOMAIN_MFG {
+							reg = <MT8173_POWER_DOMAIN_MFG>;
+							#power-domain-cells = <0>;
+							mediatek,infracfg = <&infracfg>;
+						};
+					};
+				};
+			};
 		};
 
 		watchdog: watchdog@10007000 {
@@ -792,7 +858,7 @@ afe: audio-controller@11220000  {
 			compatible = "mediatek,mt8173-afe-pcm";
 			reg = <0 0x11220000 0 0x1000>;
 			interrupts = <GIC_SPI 134 IRQ_TYPE_EDGE_FALLING>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_AUDIO>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_AUDIO>;
 			clocks = <&infracfg CLK_INFRA_AUDIO>,
 				 <&topckgen CLK_TOP_AUDIO_SEL>,
 				 <&topckgen CLK_TOP_AUD_INTBUS_SEL>,
@@ -868,7 +934,7 @@ ssusb: usb@11271000 {
 			phys = <&u2port0 PHY_TYPE_USB2>,
 			       <&u3port0 PHY_TYPE_USB3>,
 			       <&u2port1 PHY_TYPE_USB2>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_USB>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_USB>;
 			clocks = <&topckgen CLK_TOP_USB30_SEL>, <&clk26m>;
 			clock-names = "sys_ck", "ref_ck";
 			mediatek,syscon-wakeup = <&pericfg 0x400 1>;
@@ -882,7 +948,7 @@ usb_host: xhci@11270000 {
 				reg = <0 0x11270000 0 0x1000>;
 				reg-names = "mac";
 				interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_LOW>;
-				power-domains = <&scpsys MT8173_POWER_DOMAIN_USB>;
+				power-domains = <&spm MT8173_POWER_DOMAIN_USB>;
 				clocks = <&topckgen CLK_TOP_USB30_SEL>, <&clk26m>;
 				clock-names = "sys_ck", "ref_ck";
 				status = "disabled";
@@ -925,7 +991,7 @@ u2port1: usb-phy@11291000 {
 		mmsys: syscon@14000000 {
 			compatible = "mediatek,mt8173-mmsys", "syscon";
 			reg = <0 0x14000000 0 0x1000>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			assigned-clocks = <&topckgen CLK_TOP_MM_SEL>;
 			assigned-clock-rates = <400000000>;
 			#clock-cells = <1>;
@@ -940,7 +1006,7 @@ mdp_rdma0: rdma@14001000 {
 			reg = <0 0x14001000 0 0x1000>;
 			clocks = <&mmsys CLK_MM_MDP_RDMA0>,
 				 <&mmsys CLK_MM_MUTEX_32K>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			iommus = <&iommu M4U_PORT_MDP_RDMA0>;
 			mediatek,larb = <&larb0>;
 			mediatek,vpu = <&vpu>;
@@ -951,7 +1017,7 @@ mdp_rdma1: rdma@14002000 {
 			reg = <0 0x14002000 0 0x1000>;
 			clocks = <&mmsys CLK_MM_MDP_RDMA1>,
 				 <&mmsys CLK_MM_MUTEX_32K>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			iommus = <&iommu M4U_PORT_MDP_RDMA1>;
 			mediatek,larb = <&larb4>;
 		};
@@ -960,28 +1026,28 @@ mdp_rsz0: rsz@14003000 {
 			compatible = "mediatek,mt8173-mdp-rsz";
 			reg = <0 0x14003000 0 0x1000>;
 			clocks = <&mmsys CLK_MM_MDP_RSZ0>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 		};
 
 		mdp_rsz1: rsz@14004000 {
 			compatible = "mediatek,mt8173-mdp-rsz";
 			reg = <0 0x14004000 0 0x1000>;
 			clocks = <&mmsys CLK_MM_MDP_RSZ1>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 		};
 
 		mdp_rsz2: rsz@14005000 {
 			compatible = "mediatek,mt8173-mdp-rsz";
 			reg = <0 0x14005000 0 0x1000>;
 			clocks = <&mmsys CLK_MM_MDP_RSZ2>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 		};
 
 		mdp_wdma0: wdma@14006000 {
 			compatible = "mediatek,mt8173-mdp-wdma";
 			reg = <0 0x14006000 0 0x1000>;
 			clocks = <&mmsys CLK_MM_MDP_WDMA>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			iommus = <&iommu M4U_PORT_MDP_WDMA>;
 			mediatek,larb = <&larb0>;
 		};
@@ -990,7 +1056,7 @@ mdp_wrot0: wrot@14007000 {
 			compatible = "mediatek,mt8173-mdp-wrot";
 			reg = <0 0x14007000 0 0x1000>;
 			clocks = <&mmsys CLK_MM_MDP_WROT0>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			iommus = <&iommu M4U_PORT_MDP_WROT0>;
 			mediatek,larb = <&larb0>;
 		};
@@ -999,7 +1065,7 @@ mdp_wrot1: wrot@14008000 {
 			compatible = "mediatek,mt8173-mdp-wrot";
 			reg = <0 0x14008000 0 0x1000>;
 			clocks = <&mmsys CLK_MM_MDP_WROT1>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			iommus = <&iommu M4U_PORT_MDP_WROT1>;
 			mediatek,larb = <&larb4>;
 		};
@@ -1008,7 +1074,7 @@ ovl0: ovl@1400c000 {
 			compatible = "mediatek,mt8173-disp-ovl";
 			reg = <0 0x1400c000 0 0x1000>;
 			interrupts = <GIC_SPI 180 IRQ_TYPE_LEVEL_LOW>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			clocks = <&mmsys CLK_MM_DISP_OVL0>;
 			iommus = <&iommu M4U_PORT_DISP_OVL0>;
 			mediatek,larb = <&larb0>;
@@ -1019,7 +1085,7 @@ ovl1: ovl@1400d000 {
 			compatible = "mediatek,mt8173-disp-ovl";
 			reg = <0 0x1400d000 0 0x1000>;
 			interrupts = <GIC_SPI 181 IRQ_TYPE_LEVEL_LOW>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			clocks = <&mmsys CLK_MM_DISP_OVL1>;
 			iommus = <&iommu M4U_PORT_DISP_OVL1>;
 			mediatek,larb = <&larb4>;
@@ -1030,7 +1096,7 @@ rdma0: rdma@1400e000 {
 			compatible = "mediatek,mt8173-disp-rdma";
 			reg = <0 0x1400e000 0 0x1000>;
 			interrupts = <GIC_SPI 182 IRQ_TYPE_LEVEL_LOW>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			clocks = <&mmsys CLK_MM_DISP_RDMA0>;
 			iommus = <&iommu M4U_PORT_DISP_RDMA0>;
 			mediatek,larb = <&larb0>;
@@ -1041,7 +1107,7 @@ rdma1: rdma@1400f000 {
 			compatible = "mediatek,mt8173-disp-rdma";
 			reg = <0 0x1400f000 0 0x1000>;
 			interrupts = <GIC_SPI 183 IRQ_TYPE_LEVEL_LOW>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			clocks = <&mmsys CLK_MM_DISP_RDMA1>;
 			iommus = <&iommu M4U_PORT_DISP_RDMA1>;
 			mediatek,larb = <&larb4>;
@@ -1052,7 +1118,7 @@ rdma2: rdma@14010000 {
 			compatible = "mediatek,mt8173-disp-rdma";
 			reg = <0 0x14010000 0 0x1000>;
 			interrupts = <GIC_SPI 184 IRQ_TYPE_LEVEL_LOW>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			clocks = <&mmsys CLK_MM_DISP_RDMA2>;
 			iommus = <&iommu M4U_PORT_DISP_RDMA2>;
 			mediatek,larb = <&larb4>;
@@ -1063,7 +1129,7 @@ wdma0: wdma@14011000 {
 			compatible = "mediatek,mt8173-disp-wdma";
 			reg = <0 0x14011000 0 0x1000>;
 			interrupts = <GIC_SPI 185 IRQ_TYPE_LEVEL_LOW>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			clocks = <&mmsys CLK_MM_DISP_WDMA0>;
 			iommus = <&iommu M4U_PORT_DISP_WDMA0>;
 			mediatek,larb = <&larb0>;
@@ -1074,7 +1140,7 @@ wdma1: wdma@14012000 {
 			compatible = "mediatek,mt8173-disp-wdma";
 			reg = <0 0x14012000 0 0x1000>;
 			interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_LOW>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			clocks = <&mmsys CLK_MM_DISP_WDMA1>;
 			iommus = <&iommu M4U_PORT_DISP_WDMA1>;
 			mediatek,larb = <&larb4>;
@@ -1085,7 +1151,7 @@ color0: color@14013000 {
 			compatible = "mediatek,mt8173-disp-color";
 			reg = <0 0x14013000 0 0x1000>;
 			interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_LOW>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			clocks = <&mmsys CLK_MM_DISP_COLOR0>;
 			mediatek,gce-client-reg = <&gce SUBSYS_1401XXXX 0x3000 0x1000>;
 		};
@@ -1094,7 +1160,7 @@ color1: color@14014000 {
 			compatible = "mediatek,mt8173-disp-color";
 			reg = <0 0x14014000 0 0x1000>;
 			interrupts = <GIC_SPI 188 IRQ_TYPE_LEVEL_LOW>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			clocks = <&mmsys CLK_MM_DISP_COLOR1>;
 			mediatek,gce-client-reg = <&gce SUBSYS_1401XXXX 0x4000 0x1000>;
 		};
@@ -1103,7 +1169,7 @@ aal@14015000 {
 			compatible = "mediatek,mt8173-disp-aal";
 			reg = <0 0x14015000 0 0x1000>;
 			interrupts = <GIC_SPI 189 IRQ_TYPE_LEVEL_LOW>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			clocks = <&mmsys CLK_MM_DISP_AAL>;
 			mediatek,gce-client-reg = <&gce SUBSYS_1401XXXX 0x5000 0x1000>;
 		};
@@ -1112,7 +1178,7 @@ gamma@14016000 {
 			compatible = "mediatek,mt8173-disp-gamma";
 			reg = <0 0x14016000 0 0x1000>;
 			interrupts = <GIC_SPI 190 IRQ_TYPE_LEVEL_LOW>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			clocks = <&mmsys CLK_MM_DISP_GAMMA>;
 			mediatek,gce-client-reg = <&gce SUBSYS_1401XXXX 0x6000 0x1000>;
 		};
@@ -1120,21 +1186,21 @@ gamma@14016000 {
 		merge@14017000 {
 			compatible = "mediatek,mt8173-disp-merge";
 			reg = <0 0x14017000 0 0x1000>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			clocks = <&mmsys CLK_MM_DISP_MERGE>;
 		};
 
 		split0: split@14018000 {
 			compatible = "mediatek,mt8173-disp-split";
 			reg = <0 0x14018000 0 0x1000>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			clocks = <&mmsys CLK_MM_DISP_SPLIT0>;
 		};
 
 		split1: split@14019000 {
 			compatible = "mediatek,mt8173-disp-split";
 			reg = <0 0x14019000 0 0x1000>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			clocks = <&mmsys CLK_MM_DISP_SPLIT1>;
 		};
 
@@ -1142,7 +1208,7 @@ ufoe@1401a000 {
 			compatible = "mediatek,mt8173-disp-ufoe";
 			reg = <0 0x1401a000 0 0x1000>;
 			interrupts = <GIC_SPI 191 IRQ_TYPE_LEVEL_LOW>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			clocks = <&mmsys CLK_MM_DISP_UFOE>;
 		};
 
@@ -1150,7 +1216,7 @@ dsi0: dsi@1401b000 {
 			compatible = "mediatek,mt8173-dsi";
 			reg = <0 0x1401b000 0 0x1000>;
 			interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_LOW>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			clocks = <&mmsys CLK_MM_DSI0_ENGINE>,
 				 <&mmsys CLK_MM_DSI0_DIGITAL>,
 				 <&mipi_tx0>;
@@ -1164,7 +1230,7 @@ dsi1: dsi@1401c000 {
 			compatible = "mediatek,mt8173-dsi";
 			reg = <0 0x1401c000 0 0x1000>;
 			interrupts = <GIC_SPI 193 IRQ_TYPE_LEVEL_LOW>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			clocks = <&mmsys CLK_MM_DSI1_ENGINE>,
 				 <&mmsys CLK_MM_DSI1_DIGITAL>,
 				 <&mipi_tx1>;
@@ -1178,7 +1244,7 @@ dpi0: dpi@1401d000 {
 			compatible = "mediatek,mt8173-dpi";
 			reg = <0 0x1401d000 0 0x1000>;
 			interrupts = <GIC_SPI 194 IRQ_TYPE_LEVEL_LOW>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			clocks = <&mmsys CLK_MM_DPI_PIXEL>,
 				 <&mmsys CLK_MM_DPI_ENGINE>,
 				 <&apmixedsys CLK_APMIXED_TVDPLL>;
@@ -1218,7 +1284,7 @@ mutex: mutex@14020000 {
 			compatible = "mediatek,mt8173-disp-mutex";
 			reg = <0 0x14020000 0 0x1000>;
 			interrupts = <GIC_SPI 169 IRQ_TYPE_LEVEL_LOW>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			clocks = <&mmsys CLK_MM_MUTEX_32K>;
 			mediatek,gce-events = <CMDQ_EVENT_MUTEX0_STREAM_EOF>,
                                               <CMDQ_EVENT_MUTEX1_STREAM_EOF>;
@@ -1228,7 +1294,7 @@ larb0: larb@14021000 {
 			compatible = "mediatek,mt8173-smi-larb";
 			reg = <0 0x14021000 0 0x1000>;
 			mediatek,smi = <&smi_common>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			clocks = <&mmsys CLK_MM_SMI_LARB0>,
 				 <&mmsys CLK_MM_SMI_LARB0>;
 			clock-names = "apb", "smi";
@@ -1237,7 +1303,7 @@ larb0: larb@14021000 {
 		smi_common: smi@14022000 {
 			compatible = "mediatek,mt8173-smi-common";
 			reg = <0 0x14022000 0 0x1000>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			clocks = <&mmsys CLK_MM_SMI_COMMON>,
 				 <&mmsys CLK_MM_SMI_COMMON>;
 			clock-names = "apb", "smi";
@@ -1285,7 +1351,7 @@ larb4: larb@14027000 {
 			compatible = "mediatek,mt8173-smi-larb";
 			reg = <0 0x14027000 0 0x1000>;
 			mediatek,smi = <&smi_common>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
 			clocks = <&mmsys CLK_MM_SMI_LARB4>,
 				 <&mmsys CLK_MM_SMI_LARB4>;
 			clock-names = "apb", "smi";
@@ -1301,7 +1367,7 @@ larb2: larb@15001000 {
 			compatible = "mediatek,mt8173-smi-larb";
 			reg = <0 0x15001000 0 0x1000>;
 			mediatek,smi = <&smi_common>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_ISP>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_ISP>;
 			clocks = <&imgsys CLK_IMG_LARB2_SMI>,
 				 <&imgsys CLK_IMG_LARB2_SMI>;
 			clock-names = "apb", "smi";
@@ -1338,7 +1404,7 @@ vcodec_dec: vcodec@16000000 {
 				 <&iommu M4U_PORT_HW_VDEC_VLD_EXT>,
 				 <&iommu M4U_PORT_HW_VDEC_VLD2_EXT>;
 			mediatek,vpu = <&vpu>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_VDEC>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_VDEC>;
 			clocks = <&apmixedsys CLK_APMIXED_VCODECPLL>,
 				 <&topckgen CLK_TOP_UNIVPLL_D2>,
 				 <&topckgen CLK_TOP_CCI400_SEL>,
@@ -1370,7 +1436,7 @@ larb1: larb@16010000 {
 			compatible = "mediatek,mt8173-smi-larb";
 			reg = <0 0x16010000 0 0x1000>;
 			mediatek,smi = <&smi_common>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_VDEC>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_VDEC>;
 			clocks = <&vdecsys CLK_VDEC_CKEN>,
 				 <&vdecsys CLK_VDEC_LARB_CKEN>;
 			clock-names = "apb", "smi";
@@ -1386,7 +1452,7 @@ larb3: larb@18001000 {
 			compatible = "mediatek,mt8173-smi-larb";
 			reg = <0 0x18001000 0 0x1000>;
 			mediatek,smi = <&smi_common>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_VENC>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_VENC>;
 			clocks = <&vencsys CLK_VENC_CKE1>,
 				 <&vencsys CLK_VENC_CKE0>;
 			clock-names = "apb", "smi";
@@ -1443,7 +1509,7 @@ jpegdec: jpegdec@18004000 {
 				 <&vencsys CLK_VENC_CKE3>;
 			clock-names = "jpgdec-smi",
 				      "jpgdec";
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_VENC>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_VENC>;
 			mediatek,larb = <&larb3>;
 			iommus = <&iommu M4U_PORT_JPGDEC_WDMA>,
 				 <&iommu M4U_PORT_JPGDEC_BSDMA>;
@@ -1459,7 +1525,7 @@ larb5: larb@19001000 {
 			compatible = "mediatek,mt8173-smi-larb";
 			reg = <0 0x19001000 0 0x1000>;
 			mediatek,smi = <&smi_common>;
-			power-domains = <&scpsys MT8173_POWER_DOMAIN_VENC_LT>;
+			power-domains = <&spm MT8173_POWER_DOMAIN_VENC_LT>;
 			clocks = <&vencltsys CLK_VENCLT_CKE1>,
 				 <&vencltsys CLK_VENCLT_CKE0>;
 			clock-names = "apb", "smi";
-- 
2.28.0


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

* [PATCH v2 04/12] soc: mediatek: pm-domains: Add bus protection protocol
  2020-10-01 16:01 [PATCH v2 00/12] soc: mediatek: pm-domains: Add new driver for SCPSYS power domains controller Enric Balletbo i Serra
                   ` (2 preceding siblings ...)
  2020-10-01 16:01 ` [PATCH v2 03/12] arm64: dts: mediatek: Add mt8173 power domain controller Enric Balletbo i Serra
@ 2020-10-01 16:01 ` Enric Balletbo i Serra
  2020-10-01 16:01 ` [PATCH v2 05/12] soc: mediatek: pm_domains: Make bus protection generic Enric Balletbo i Serra
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 30+ messages in thread
From: Enric Balletbo i Serra @ 2020-10-01 16:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Collabora Kernel ML, fparent, matthias.bgg, drinkcat, hsinyi,
	weiyi.lu, Matthias Brugger, linux-arm-kernel, linux-mediatek

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

Changes in v2: None

 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 68886bf437f9..cc8915b53c7e 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -54,6 +54,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;
@@ -74,7 +76,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 {
@@ -162,24 +164,40 @@ static int scpsys_sram_disable(struct scpsys_domain *pd)
 
 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)
@@ -479,7 +497,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,
@@ -521,7 +539,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


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

* [PATCH v2 05/12] soc: mediatek: pm_domains: Make bus protection generic
  2020-10-01 16:01 [PATCH v2 00/12] soc: mediatek: pm-domains: Add new driver for SCPSYS power domains controller Enric Balletbo i Serra
                   ` (3 preceding siblings ...)
  2020-10-01 16:01 ` [PATCH v2 04/12] soc: mediatek: pm-domains: Add bus protection protocol Enric Balletbo i Serra
@ 2020-10-01 16:01 ` Enric Balletbo i Serra
  2020-10-01 16:01 ` [PATCH v2 06/12] soc: mediatek: pm-domains: Add SMI block as bus protection block Enric Balletbo i Serra
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 30+ messages in thread
From: Enric Balletbo i Serra @ 2020-10-01 16:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Collabora Kernel ML, fparent, matthias.bgg, drinkcat, hsinyi,
	weiyi.lu, Matthias Brugger, linux-arm-kernel, linux-mediatek

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

Changes in v2: None

 drivers/soc/mediatek/mtk-infracfg.c   |  5 ---
 drivers/soc/mediatek/mtk-pm-domains.c | 53 +++++++++++++++++++++------
 include/linux/soc/mediatek/infracfg.h |  5 +++
 3 files changed, 47 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 cc8915b53c7e..b5e7c9846c34 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -162,18 +162,24 @@ static int scpsys_sram_disable(struct scpsys_domain *pd)
 					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;
 	}
@@ -181,18 +187,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;
 	}
@@ -200,6 +222,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


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

* [PATCH v2 06/12] soc: mediatek: pm-domains: Add SMI block as bus protection block
  2020-10-01 16:01 [PATCH v2 00/12] soc: mediatek: pm-domains: Add new driver for SCPSYS power domains controller Enric Balletbo i Serra
                   ` (4 preceding siblings ...)
  2020-10-01 16:01 ` [PATCH v2 05/12] soc: mediatek: pm_domains: Make bus protection generic Enric Balletbo i Serra
@ 2020-10-01 16:01 ` Enric Balletbo i Serra
  2020-10-02  8:56   ` Matthias Brugger
  2020-10-01 16:01 ` [PATCH v2 07/12] soc: mediatek: pm-domains: Add extra sram control Enric Balletbo i Serra
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 30+ messages in thread
From: Enric Balletbo i Serra @ 2020-10-01 16:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Collabora Kernel ML, fparent, matthias.bgg, drinkcat, hsinyi,
	weiyi.lu, Matthias Brugger, linux-arm-kernel, linux-mediatek

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

Changes in v2: None

 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 b5e7c9846c34..38f2630bdd0a 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 {
@@ -175,9 +195,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)
@@ -193,7 +213,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,
@@ -208,11 +232,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)
@@ -228,7 +252,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)
@@ -358,6 +386,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);
@@ -528,10 +560,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] = {
@@ -570,12 +601,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


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

* [PATCH v2 07/12] soc: mediatek: pm-domains: Add extra sram control
  2020-10-01 16:01 [PATCH v2 00/12] soc: mediatek: pm-domains: Add new driver for SCPSYS power domains controller Enric Balletbo i Serra
                   ` (5 preceding siblings ...)
  2020-10-01 16:01 ` [PATCH v2 06/12] soc: mediatek: pm-domains: Add SMI block as bus protection block Enric Balletbo i Serra
@ 2020-10-01 16:01 ` Enric Balletbo i Serra
  2020-10-02  9:24   ` Matthias Brugger
  2020-10-01 16:01 ` [PATCH v2 08/12] soc: mediatek: pm-domains: Add subsystem clocks Enric Balletbo i Serra
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 30+ messages in thread
From: Enric Balletbo i Serra @ 2020-10-01 16:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Collabora Kernel ML, fparent, matthias.bgg, drinkcat, hsinyi,
	weiyi.lu, Matthias Brugger, linux-arm-kernel, linux-mediatek

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

Changes in v2:
- Nit, split readl(ctl_addr) | pd->data->sram_pdn_bits in two lines.
- Use regmap API

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

diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index 38f2630bdd0a..e0a52d489fea 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)
@@ -155,14 +158,28 @@ static int scpsys_sram_enable(struct scpsys_domain *pd)
 	struct scpsys *scpsys = pd->scpsys;
 	u32 val;
 	int tmp;
+	int ret;
 
 	regmap_read(scpsys->base, pd->data->ctl_offs, &val);
 	val &= ~pd->data->sram_pdn_bits;
 	regmap_write(scpsys->base, pd->data->ctl_offs, val);
 
 	/* Either wait until SRAM_PDN_ACK all 1 or 0 */
-	return regmap_read_poll_timeout(scpsys->base, pd->data->ctl_offs, tmp,
-					(tmp & pdn_ack) == 0, MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
+	ret = regmap_read_poll_timeout(scpsys->base, pd->data->ctl_offs, tmp,
+				       (tmp & pdn_ack) == 0, MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
+	if (ret < 0)
+		return ret;
+
+	if (MTK_SCPD_CAPS(pd, MTK_SCPD_SRAM_ISO)) {
+		regmap_read(scpsys->base, pd->data->ctl_offs, &val);
+		val |= PWR_SRAM_ISOINT_B_BIT;
+		regmap_write(scpsys->base, pd->data->ctl_offs, val);
+		udelay(1);
+		val &= ~PWR_SRAM_CLKISO_BIT;
+		regmap_write(scpsys->base, pd->data->ctl_offs, val);
+	}
+
+	return 0;
 }
 
 static int scpsys_sram_disable(struct scpsys_domain *pd)
@@ -172,6 +189,15 @@ static int scpsys_sram_disable(struct scpsys_domain *pd)
 	u32 val;
 	int tmp;
 
+	if (MTK_SCPD_CAPS(pd, MTK_SCPD_SRAM_ISO)) {
+		regmap_read(scpsys->base, pd->data->ctl_offs, &val);
+		val |= PWR_SRAM_CLKISO_BIT;
+		regmap_write(scpsys->base, pd->data->ctl_offs, val);
+		val &= ~PWR_SRAM_ISOINT_B_BIT;
+		regmap_write(scpsys->base, pd->data->ctl_offs, val);
+		udelay(1);
+	}
+
 	regmap_read(scpsys->base, pd->data->ctl_offs, &val);
 	val |= pd->data->sram_pdn_bits;
 	regmap_write(scpsys->base, pd->data->ctl_offs, val);
-- 
2.28.0


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

* [PATCH v2 08/12] soc: mediatek: pm-domains: Add subsystem clocks
  2020-10-01 16:01 [PATCH v2 00/12] soc: mediatek: pm-domains: Add new driver for SCPSYS power domains controller Enric Balletbo i Serra
                   ` (6 preceding siblings ...)
  2020-10-01 16:01 ` [PATCH v2 07/12] soc: mediatek: pm-domains: Add extra sram control Enric Balletbo i Serra
@ 2020-10-01 16:01 ` Enric Balletbo i Serra
  2020-10-02  9:04   ` Matthias Brugger
  2020-10-01 16:01 ` [PATCH v2 09/12] soc: mediatek: pm-domains: Allow bus protection to ignore clear ack Enric Balletbo i Serra
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 30+ messages in thread
From: Enric Balletbo i Serra @ 2020-10-01 16:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Collabora Kernel ML, fparent, matthias.bgg, drinkcat, hsinyi,
	weiyi.lu, Matthias Brugger, linux-arm-kernel, linux-mediatek

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

Changes in v2:
- Use dev_err_probe if getting clocks fails, so an error is not printed
  if is deferred.

 drivers/soc/mediatek/mtk-pm-domains.c | 89 ++++++++++++++++++++++-----
 1 file changed, 75 insertions(+), 14 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index e0a52d489fea..2075072f16da 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;
 };
@@ -318,16 +323,22 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
 	val |= PWR_RST_B_BIT;
 	regmap_write(scpsys->base, pd->data->ctl_offs, val);
 
+	ret = clk_bulk_enable(pd->num_subsys_clks, pd->subsys_clks);
+	if (ret)
+		goto err_pwr_ack;
+
 	ret = scpsys_sram_enable(pd);
 	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);
 	return ret;
@@ -348,6 +359,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 */
 	regmap_read(scpsys->base, pd->data->ctl_offs, &val);
 	val |= PWR_ISO_BIT;
@@ -380,8 +393,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) {
@@ -416,28 +432,63 @@ 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);
-			dev_err_probe(scpsys->dev, ret, "%pOFn: failed to get clk at index %d\n",
-				      node, i);
-			return ret;
+		struct clk *clk = of_clk_get(node, i);
+
+		if (IS_ERR(clk)) {
+			ret = PTR_ERR(clk);
+			dev_err_probe(scpsys->dev, PTR_ERR(clk),
+				      "%pOFn: failed to get clk at index %d\n", node, i);
+			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_probe(scpsys->dev, PTR_ERR(clk),
+				      "%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
@@ -462,6 +513,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);
@@ -541,6 +598,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


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

* [PATCH v2 09/12] soc: mediatek: pm-domains: Allow bus protection to ignore clear ack
  2020-10-01 16:01 [PATCH v2 00/12] soc: mediatek: pm-domains: Add new driver for SCPSYS power domains controller Enric Balletbo i Serra
                   ` (7 preceding siblings ...)
  2020-10-01 16:01 ` [PATCH v2 08/12] soc: mediatek: pm-domains: Add subsystem clocks Enric Balletbo i Serra
@ 2020-10-01 16:01 ` Enric Balletbo i Serra
  2020-10-01 16:01 ` [PATCH v2 10/12] dt-bindings: power: Add MT8183 power domains Enric Balletbo i Serra
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 30+ messages in thread
From: Enric Balletbo i Serra @ 2020-10-01 16:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Collabora Kernel ML, fparent, matthias.bgg, drinkcat, hsinyi,
	weiyi.lu, Matthias Brugger, linux-arm-kernel, linux-mediatek

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

Changes in v2: None

 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 2075072f16da..0222be12d0df 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
@@ -267,6 +272,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


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

* [PATCH v2 10/12] dt-bindings: power: Add MT8183 power domains
  2020-10-01 16:01 [PATCH v2 00/12] soc: mediatek: pm-domains: Add new driver for SCPSYS power domains controller Enric Balletbo i Serra
                   ` (8 preceding siblings ...)
  2020-10-01 16:01 ` [PATCH v2 09/12] soc: mediatek: pm-domains: Allow bus protection to ignore clear ack Enric Balletbo i Serra
@ 2020-10-01 16:01 ` Enric Balletbo i Serra
  2020-10-01 16:01 ` [PATCH v2 11/12] soc: mediatek: pm-domains: Add support for mt8183 Enric Balletbo i Serra
  2020-10-01 16:01 ` [PATCH v2 12/12] arm64: dts: mediatek: Add mt8183 power domains controller Enric Balletbo i Serra
  11 siblings, 0 replies; 30+ messages in thread
From: Enric Balletbo i Serra @ 2020-10-01 16:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Collabora Kernel ML, fparent, matthias.bgg, drinkcat, hsinyi,
	weiyi.lu, Matthias Brugger, Rob Herring, devicetree,
	linux-arm-kernel, linux-mediatek

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

Changes in v2: None

 .../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 9ac7a7694d61..67a15f292e0a 100644
--- a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
+++ b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
@@ -24,6 +24,7 @@ properties:
   compatible:
     enum:
       - mediatek,mt8173-power-controller
+      - mediatek,mt8183-power-controller
 
   '#power-domain-cells':
     const: 1
@@ -58,6 +59,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
 
       clocks:
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


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

* [PATCH v2 11/12] soc: mediatek: pm-domains: Add support for mt8183
  2020-10-01 16:01 [PATCH v2 00/12] soc: mediatek: pm-domains: Add new driver for SCPSYS power domains controller Enric Balletbo i Serra
                   ` (9 preceding siblings ...)
  2020-10-01 16:01 ` [PATCH v2 10/12] dt-bindings: power: Add MT8183 power domains Enric Balletbo i Serra
@ 2020-10-01 16:01 ` Enric Balletbo i Serra
  2020-10-01 16:01 ` [PATCH v2 12/12] arm64: dts: mediatek: Add mt8183 power domains controller Enric Balletbo i Serra
  11 siblings, 0 replies; 30+ messages in thread
From: Enric Balletbo i Serra @ 2020-10-01 16:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Collabora Kernel ML, fparent, matthias.bgg, drinkcat, hsinyi,
	weiyi.lu, Matthias Brugger, linux-arm-kernel, linux-mediatek

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

Changes in v2:
- Do not use hardcoded values for triplets set, clear and status in
  infracfg and SMI.

 drivers/soc/mediatek/mtk-pm-domains.c | 217 ++++++++++++++++++++++++++
 include/linux/soc/mediatek/infracfg.h |  46 ++++++
 2 files changed, 263 insertions(+)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index 0222be12d0df..b7ccf94c1c58 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)
@@ -705,6 +707,210 @@ 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, MT8183_TOP_AXI_PROT_EN_SET,
+				    MT8183_TOP_AXI_PROT_EN_CLR, MT8183_TOP_AXI_PROT_EN_STA1),
+		},
+	},
+	[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, MT8183_TOP_AXI_PROT_EN_1_SET,
+				    MT8183_TOP_AXI_PROT_EN_1_CLR, MT8183_TOP_AXI_PROT_EN_STA1_1),
+			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MFG, MT8183_TOP_AXI_PROT_EN_SET,
+				    MT8183_TOP_AXI_PROT_EN_CLR, MT8183_TOP_AXI_PROT_EN_STA1),
+		},
+	},
+	[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, MT8183_TOP_AXI_PROT_EN_1_SET,
+				    MT8183_TOP_AXI_PROT_EN_1_CLR, MT8183_TOP_AXI_PROT_EN_STA1_1),
+			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_DISP, MT8183_TOP_AXI_PROT_EN_SET,
+				    MT8183_TOP_AXI_PROT_EN_CLR, MT8183_TOP_AXI_PROT_EN_STA1),
+		},
+		.bp_smi = {
+			BUS_PROT_WR(MT8183_SMI_COMMON_SMI_CLAMP_DISP,
+				    MT8183_SMI_COMMON_CLAMP_EN_SET,
+				    MT8183_SMI_COMMON_CLAMP_EN_CLR,
+				    MT8183_SMI_COMMON_CLAMP_EN),
+		},
+	},
+	[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, MT8183_TOP_AXI_PROT_EN_MM_SET,
+				    MT8183_TOP_AXI_PROT_EN_MM_CLR, MT8183_TOP_AXI_PROT_EN_MM_STA1),
+			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_CAM, MT8183_TOP_AXI_PROT_EN_SET,
+				    MT8183_TOP_AXI_PROT_EN_CLR, MT8183_TOP_AXI_PROT_EN_STA1),
+			BUS_PROT_WR_IGN(MT8183_TOP_AXI_PROT_EN_MM_CAM_2ND,
+					MT8183_TOP_AXI_PROT_EN_MM_SET,
+					MT8183_TOP_AXI_PROT_EN_MM_CLR,
+					MT8183_TOP_AXI_PROT_EN_MM_STA1),
+		},
+		.bp_smi = {
+			BUS_PROT_WR(MT8183_SMI_COMMON_SMI_CLAMP_CAM,
+				    MT8183_SMI_COMMON_CLAMP_EN_SET,
+				    MT8183_SMI_COMMON_CLAMP_EN_CLR,
+				    MT8183_SMI_COMMON_CLAMP_EN),
+		},
+	},
+	[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,
+				    MT8183_TOP_AXI_PROT_EN_MM_SET,
+				    MT8183_TOP_AXI_PROT_EN_MM_CLR,
+				    MT8183_TOP_AXI_PROT_EN_MM_STA1),
+			BUS_PROT_WR_IGN(MT8183_TOP_AXI_PROT_EN_MM_ISP_2ND,
+					MT8183_TOP_AXI_PROT_EN_MM_SET,
+					MT8183_TOP_AXI_PROT_EN_MM_CLR,
+					MT8183_TOP_AXI_PROT_EN_MM_STA1),
+		},
+		.bp_smi = {
+			BUS_PROT_WR(MT8183_SMI_COMMON_SMI_CLAMP_ISP,
+				    MT8183_SMI_COMMON_CLAMP_EN_SET,
+				    MT8183_SMI_COMMON_CLAMP_EN_CLR,
+				    MT8183_SMI_COMMON_CLAMP_EN),
+		},
+	},
+	[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,
+				    MT8183_SMI_COMMON_CLAMP_EN_SET,
+				    MT8183_SMI_COMMON_CLAMP_EN_CLR,
+				    MT8183_SMI_COMMON_CLAMP_EN),
+		},
+	},
+	[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,
+				    MT8183_SMI_COMMON_CLAMP_EN_SET,
+				    MT8183_SMI_COMMON_CLAMP_EN_CLR,
+				    MT8183_SMI_COMMON_CLAMP_EN),
+		},
+	},
+	[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,
+				    MT8183_TOP_AXI_PROT_EN_MM_SET,
+				    MT8183_TOP_AXI_PROT_EN_MM_CLR,
+				    MT8183_TOP_AXI_PROT_EN_MM_STA1),
+			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_VPU_TOP,
+				    MT8183_TOP_AXI_PROT_EN_SET,
+				    MT8183_TOP_AXI_PROT_EN_CLR,
+				    MT8183_TOP_AXI_PROT_EN_STA1),
+			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MM_VPU_TOP_2ND,
+				    MT8183_TOP_AXI_PROT_EN_MM_SET,
+				    MT8183_TOP_AXI_PROT_EN_MM_CLR,
+				    MT8183_TOP_AXI_PROT_EN_MM_STA1),
+		},
+		.bp_smi = {
+			BUS_PROT_WR(MT8183_SMI_COMMON_SMI_CLAMP_VPU_TOP,
+				    MT8183_SMI_COMMON_CLAMP_EN_SET,
+				    MT8183_SMI_COMMON_CLAMP_EN_CLR,
+				    MT8183_SMI_COMMON_CLAMP_EN),
+		},
+	},
+	[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,
+				    MT8183_TOP_AXI_PROT_EN_MCU_SET,
+				    MT8183_TOP_AXI_PROT_EN_MCU_CLR,
+				    MT8183_TOP_AXI_PROT_EN_MCU_STA1),
+			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MCU_VPU_CORE0_2ND,
+				    MT8183_TOP_AXI_PROT_EN_MCU_SET,
+				    MT8183_TOP_AXI_PROT_EN_MCU_CLR,
+				    MT8183_TOP_AXI_PROT_EN_MCU_STA1),
+		},
+		.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,
+				    MT8183_TOP_AXI_PROT_EN_MCU_SET,
+				    MT8183_TOP_AXI_PROT_EN_MCU_CLR,
+				    MT8183_TOP_AXI_PROT_EN_MCU_STA1),
+			BUS_PROT_WR(MT8183_TOP_AXI_PROT_EN_MCU_VPU_CORE1_2ND,
+				    MT8183_TOP_AXI_PROT_EN_MCU_SET,
+				    MT8183_TOP_AXI_PROT_EN_MCU_CLR,
+				    MT8183_TOP_AXI_PROT_EN_MCU_STA1),
+		},
+		.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),
@@ -712,11 +918,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..826ed51c4843 100644
--- a/include/linux/soc/mediatek/infracfg.h
+++ b/include/linux/soc/mediatek/infracfg.h
@@ -2,6 +2,52 @@
 #ifndef __SOC_MEDIATEK_INFRACFG_H
 #define __SOC_MEDIATEK_INFRACFG_H
 
+#define MT8183_TOP_AXI_PROT_EN_STA1			0x228
+#define MT8183_TOP_AXI_PROT_EN_STA1_1			0x258
+#define MT8183_TOP_AXI_PROT_EN_SET			0x2a0
+#define MT8183_TOP_AXI_PROT_EN_CLR			0x2a4
+#define MT8183_TOP_AXI_PROT_EN_1_SET			0x2a8
+#define MT8183_TOP_AXI_PROT_EN_1_CLR			0x2ac
+#define MT8183_TOP_AXI_PROT_EN_MCU_SET			0x2c4
+#define MT8183_TOP_AXI_PROT_EN_MCU_CLR			0x2e4
+#define MT8183_TOP_AXI_PROT_EN_MCU_STA1			0x2ec
+#define MT8183_TOP_AXI_PROT_EN_MM_SET			0x2d4
+#define MT8183_TOP_AXI_PROT_EN_MM_CLR			0x2d8
+#define MT8183_TOP_AXI_PROT_EN_MM_STA1			0x2ec
+
+#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_CLAMP_EN			0x3c0
+#define MT8183_SMI_COMMON_CLAMP_EN_SET			0x3c4
+#define MT8183_SMI_COMMON_CLAMP_EN_CLR			0x3c8
+
+#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


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

* [PATCH v2 12/12] arm64: dts: mediatek: Add mt8183 power domains controller
  2020-10-01 16:01 [PATCH v2 00/12] soc: mediatek: pm-domains: Add new driver for SCPSYS power domains controller Enric Balletbo i Serra
                   ` (10 preceding siblings ...)
  2020-10-01 16:01 ` [PATCH v2 11/12] soc: mediatek: pm-domains: Add support for mt8183 Enric Balletbo i Serra
@ 2020-10-01 16:01 ` Enric Balletbo i Serra
  2020-10-02  9:11   ` Matthias Brugger
  2020-10-11 14:55   ` kernel test robot
  11 siblings, 2 replies; 30+ messages in thread
From: Enric Balletbo i Serra @ 2020-10-01 16:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Collabora Kernel ML, fparent, matthias.bgg, drinkcat, hsinyi,
	weiyi.lu, Matthias Brugger, Rob Herring, devicetree,
	linux-arm-kernel, linux-mediatek

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

Changes in v2: None

 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 162 +++++++++++++++++++++++
 drivers/soc/mediatek/mtk-mmsys.c         |   4 -
 2 files changed, 162 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 0e4d3e18cad5..9082bc65e15e 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,167 @@ pio: pinctrl@10005000 {
 			#interrupt-cells = <2>;
 		};
 
+		scpsys: syscon@10006000 {
+			compatible = "syscon", "simple-mfd";
+			reg = <0 0x10006000 0 0x1000>;
+			#power-domain-cells = <1>;
+
+			/* System Power Manager */
+			spm: power-controller {
+				compatible = "mediatek,mt8183-power-controller";
+				#address-cells = <1>;
+				#size-cells = <0>;
+				#power-domain-cells = <1>;
+
+				/* power domain of the SoC */
+				power-domain@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>;
+				};
+
+				power-domain@MT8183_POWER_DOMAIN_CONN {
+					reg = <MT8183_POWER_DOMAIN_CONN>;
+					mediatek,infracfg = <&infracfg>;
+					#power-domain-cells = <0>;
+				};
+
+				power-domain@MT8183_POWER_DOMAIN_MFG_ASYNC {
+					reg = <MT8183_POWER_DOMAIN_MFG_ASYNC>;
+					clocks =  <&topckgen CLK_TOP_MUX_MFG>;
+					clock-names = "mfg";
+					#address-cells = <1>;
+					#size-cells = <0>;
+					#power-domain-cells = <1>;
+
+					power-domain@MT8183_POWER_DOMAIN_MFG {
+						reg = <MT8183_POWER_DOMAIN_MFG>;
+						#address-cells = <1>;
+						#size-cells = <0>;
+						#power-domain-cells = <1>;
+
+						power-domain@MT8183_POWER_DOMAIN_MFG_CORE0 {
+							reg = <MT8183_POWER_DOMAIN_MFG_CORE0>;
+							#power-domain-cells = <0>;
+						};
+
+						power-domain@MT8183_POWER_DOMAIN_MFG_CORE1 {
+							reg = <MT8183_POWER_DOMAIN_MFG_CORE1>;
+							#power-domain-cells = <0>;
+						};
+
+						power-domain@MT8183_POWER_DOMAIN_MFG_2D {
+							reg = <MT8183_POWER_DOMAIN_MFG_2D>;
+							mediatek,infracfg = <&infracfg>;
+							#power-domain-cells = <0>;
+						};
+					};
+				};
+
+				power-domain@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";
+					mediatek,infracfg = <&infracfg>;
+					mediatek,smi = <&smi_common>;
+					#address-cells = <1>;
+					#size-cells = <0>;
+					#power-domain-cells = <1>;
+
+					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>;
+					};
+
+					power-domain@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>;
+					};
+
+					power-domain@MT8183_POWER_DOMAIN_VDEC {
+						reg = <MT8183_POWER_DOMAIN_VDEC>;
+						mediatek,smi = <&smi_common>;
+						#power-domain-cells = <0>;
+					};
+
+					power-domain@MT8183_POWER_DOMAIN_VENC {
+						reg = <MT8183_POWER_DOMAIN_VENC>;
+						mediatek,smi = <&smi_common>;
+						#power-domain-cells = <0>;
+					};
+
+					power-domain@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";
+						mediatek,infracfg = <&infracfg>;
+						mediatek,smi = <&smi_common>;
+						#address-cells = <1>;
+						#size-cells = <0>;
+						#power-domain-cells = <1>;
+
+						power-domain@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>;
+						};
+
+						power-domain@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";
diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
index ec4cc5eeabff..0c490f509655 100644
--- a/drivers/soc/mediatek/mtk-mmsys.c
+++ b/drivers/soc/mediatek/mtk-mmsys.c
@@ -560,10 +560,6 @@ static const struct of_device_id of_match_mtk_mmsys[] = {
 		.compatible = "mediatek,mt8173-mmsys",
 		.data = &mt8173_mmsys_driver_data,
 	},
-	{
-		.compatible = "mediatek,mt8183-mmsys",
-		.data = &mt8183_mmsys_driver_data,
-	},
 	{ }
 };
 
-- 
2.28.0


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

* Re: [PATCH v2 06/12] soc: mediatek: pm-domains: Add SMI block as bus protection block
  2020-10-01 16:01 ` [PATCH v2 06/12] soc: mediatek: pm-domains: Add SMI block as bus protection block Enric Balletbo i Serra
@ 2020-10-02  8:56   ` Matthias Brugger
  2020-10-05  1:48     ` Nicolas Boichat
  2020-10-26 15:18     ` Enric Balletbo i Serra
  0 siblings, 2 replies; 30+ messages in thread
From: Matthias Brugger @ 2020-10-02  8:56 UTC (permalink / raw)
  To: Enric Balletbo i Serra, linux-kernel
  Cc: Collabora Kernel ML, fparent, drinkcat, hsinyi, weiyi.lu,
	Matthias Brugger, linux-arm-kernel, linux-mediatek



On 01/10/2020 18:01, 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>
> ---
> 
> Changes in v2: None
> 
>   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 b5e7c9846c34..38f2630bdd0a 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 {
> @@ -175,9 +195,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)
> @@ -193,7 +213,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,
> @@ -208,11 +232,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)
> @@ -228,7 +252,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);

Also it seems not to break the system we should disable pd->smi first and after 
that pd->infracfg (just the other way round as we did in enable).

>   }
>   
>   static int scpsys_power_on(struct generic_pm_domain *genpd)
> @@ -358,6 +386,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);
> @@ -528,10 +560,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] = {
> @@ -570,12 +601,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)			\

While at it, could you update this to call it:
BUS_PROT_UPDATE_TOPAXI as in all the other SoCs TOPAXI is mapped to the same 
address.

Thanks!
Matthias

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

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

* Re: [PATCH v2 08/12] soc: mediatek: pm-domains: Add subsystem clocks
  2020-10-01 16:01 ` [PATCH v2 08/12] soc: mediatek: pm-domains: Add subsystem clocks Enric Balletbo i Serra
@ 2020-10-02  9:04   ` Matthias Brugger
  2020-10-26 15:17     ` Enric Balletbo i Serra
  0 siblings, 1 reply; 30+ messages in thread
From: Matthias Brugger @ 2020-10-02  9:04 UTC (permalink / raw)
  To: Enric Balletbo i Serra, linux-kernel
  Cc: Collabora Kernel ML, fparent, drinkcat, hsinyi, weiyi.lu,
	Matthias Brugger, linux-arm-kernel, linux-mediatek



On 01/10/2020 18:01, 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>
> ---
> 
> Changes in v2:
> - Use dev_err_probe if getting clocks fails, so an error is not printed
>    if is deferred.
> 
>   drivers/soc/mediatek/mtk-pm-domains.c | 89 ++++++++++++++++++++++-----
>   1 file changed, 75 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
> index e0a52d489fea..2075072f16da 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.c
> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
[...]
>   
> +	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_probe(scpsys->dev, PTR_ERR(clk),
> +				      "%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;
> +

Not sure if it really matters, but logically we should prepare the basic clocks 
before we prepare the subsystem clocks. And fix the error handling accordingly.

Regards,
Matthias

>   	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
> @@ -462,6 +513,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);
> @@ -541,6 +598,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)
> 

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

* Re: [PATCH v2 12/12] arm64: dts: mediatek: Add mt8183 power domains controller
  2020-10-01 16:01 ` [PATCH v2 12/12] arm64: dts: mediatek: Add mt8183 power domains controller Enric Balletbo i Serra
@ 2020-10-02  9:11   ` Matthias Brugger
  2020-10-11 14:55   ` kernel test robot
  1 sibling, 0 replies; 30+ messages in thread
From: Matthias Brugger @ 2020-10-02  9:11 UTC (permalink / raw)
  To: Enric Balletbo i Serra, linux-kernel
  Cc: Collabora Kernel ML, fparent, drinkcat, hsinyi, weiyi.lu,
	Matthias Brugger, Rob Herring, devicetree, linux-arm-kernel,
	linux-mediatek



On 01/10/2020 18:01, Enric Balletbo i Serra wrote:
> 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>
> ---
> 
> Changes in v2: None
> 
>   arch/arm64/boot/dts/mediatek/mt8183.dtsi | 162 +++++++++++++++++++++++
>   drivers/soc/mediatek/mtk-mmsys.c         |   4 -
>   2 files changed, 162 insertions(+), 4 deletions(-)
> 
[...]
> diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
> index ec4cc5eeabff..0c490f509655 100644
> --- a/drivers/soc/mediatek/mtk-mmsys.c
> +++ b/drivers/soc/mediatek/mtk-mmsys.c
> @@ -560,10 +560,6 @@ static const struct of_device_id of_match_mtk_mmsys[] = {
>   		.compatible = "mediatek,mt8173-mmsys",
>   		.data = &mt8173_mmsys_driver_data,
>   	},
> -	{
> -		.compatible = "mediatek,mt8183-mmsys",
> -		.data = &mt8183_mmsys_driver_data,
> -	},

Looks like an oversight from your side? At least it's not explained in the 
commit message.

Regards,
Matthias

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

* Re: [PATCH v2 07/12] soc: mediatek: pm-domains: Add extra sram control
  2020-10-01 16:01 ` [PATCH v2 07/12] soc: mediatek: pm-domains: Add extra sram control Enric Balletbo i Serra
@ 2020-10-02  9:24   ` Matthias Brugger
  0 siblings, 0 replies; 30+ messages in thread
From: Matthias Brugger @ 2020-10-02  9:24 UTC (permalink / raw)
  To: Enric Balletbo i Serra, linux-kernel
  Cc: Collabora Kernel ML, fparent, drinkcat, hsinyi, weiyi.lu,
	Matthias Brugger, linux-arm-kernel, linux-mediatek



On 01/10/2020 18:01, 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>
> ---
> 
> Changes in v2:
> - Nit, split readl(ctl_addr) | pd->data->sram_pdn_bits in two lines.

Nit: si was done in 2/12 :)

Regards,
Matthias

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

* Re: [PATCH v2 02/12] soc: mediatek: Add MediaTek SCPSYS power domains
  2020-10-01 16:01 ` [PATCH v2 02/12] soc: mediatek: Add MediaTek SCPSYS power domains Enric Balletbo i Serra
@ 2020-10-05  1:39   ` Nicolas Boichat
  2020-10-26 15:17     ` Enric Balletbo i Serra
  0 siblings, 1 reply; 30+ messages in thread
From: Nicolas Boichat @ 2020-10-05  1:39 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: lkml, Collabora Kernel ML, Fabien Parent, Matthias Brugger,
	Hsin-Yi Wang, Weiyi Lu, Matthias Brugger, linux-arm Mailing List,
	moderated list:ARM/Mediatek SoC support

On Fri, Oct 2, 2020 at 12:02 AM Enric Balletbo i Serra
<enric.balletbo@collabora.com> 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>
> ---
>
> Changes in v2:
> - Get base address from parent syscon. We have now a scpsys syscon node
>   and a child for the SPM (System Power Manager).
> - Use regmap API to acces de base address.
>
>  drivers/soc/mediatek/Kconfig          |  13 +
>  drivers/soc/mediatek/Makefile         |   1 +
>  drivers/soc/mediatek/mtk-pm-domains.c | 628 ++++++++++++++++++++++++++
>  3 files changed, 642 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..68886bf437f9
> --- /dev/null
> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> @@ -0,0 +1,628 @@
> [snip]
> +
> +static int scpsys_domain_is_on(struct scpsys_domain *pd)
> +{
> +       struct scpsys *scpsys = pd->scpsys;
> +       u32 status, status2;
> +
> +       regmap_read(scpsys->base, scpsys->soc_data->pwr_sta_offs, &status);
> +       status &= pd->data->sta_mask;
> +
> +       regmap_read(scpsys->base, scpsys->soc_data->pwr_sta2nd_offs, &status2);
> +       status2 &= 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;


Why do you care? It seems like all you ever do with this is check if
the domain is powered on, so it seems like you could just
return (status && status2); ?

> +}
> +
> +static int scpsys_sram_enable(struct scpsys_domain *pd)
> +{
> +       u32 pdn_ack = pd->data->sram_pdn_ack_bits;
> +       struct scpsys *scpsys = pd->scpsys;
> +       u32 val;
> +       int tmp;

regmap_read_poll_timeout => regmap_read technically takes a `unsigned
int *` parameter, so this should probably be unsigned too (or... just
reuse val?)

> +
> +       regmap_read(scpsys->base, pd->data->ctl_offs, &val);
> +       val &= ~pd->data->sram_pdn_bits;
> +       regmap_write(scpsys->base, pd->data->ctl_offs, val);

Replace with regmap_update_bits?

> +
> +       /* Either wait until SRAM_PDN_ACK all 1 or 0 */
> +       return regmap_read_poll_timeout(scpsys->base, pd->data->ctl_offs, tmp,
> +                                       (tmp & pdn_ack) == 0, MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> +}
> +
> +static int scpsys_sram_disable(struct scpsys_domain *pd)
> +{
> +       u32 pdn_ack = pd->data->sram_pdn_ack_bits;
> +       struct scpsys *scpsys = pd->scpsys;
> +       u32 val;
> +       int tmp;
> +
> +       regmap_read(scpsys->base, pd->data->ctl_offs, &val);
> +       val |= pd->data->sram_pdn_bits;
> +       regmap_write(scpsys->base, pd->data->ctl_offs, val);
> +
> +       /* Either wait until SRAM_PDN_ACK all 1 or 0 */
> +       return regmap_read_poll_timeout(scpsys->base, pd->data->ctl_offs, 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;
> +       int ret, tmp;
> +       u32 val;
> +
> +       ret = clk_bulk_enable(pd->num_clks, pd->clks);
> +       if (ret)
> +               return ret;
> +
> +       /* subsys power on */
> +       regmap_read(scpsys->base, pd->data->ctl_offs, &val);
> +       val |= PWR_ON_BIT;
> +       regmap_write(scpsys->base, pd->data->ctl_offs, val);
> +       val |= PWR_ON_2ND_BIT;
> +       regmap_write(scpsys->base, pd->data->ctl_offs, val);
> +
> +       /* 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;
> +       regmap_write(scpsys->base, pd->data->ctl_offs, val);
> +
> +       val &= ~PWR_ISO_BIT;
> +       regmap_write(scpsys->base, pd->data->ctl_offs, val);
> +
> +       val |= PWR_RST_B_BIT;
> +       regmap_write(scpsys->base, pd->data->ctl_offs, val);
> +
> +       ret = scpsys_sram_enable(pd);
> +       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);
> +       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;
> +       int ret, tmp;
> +       u32 val;
> +
> +       ret = scpsys_bus_protect_enable(pd);
> +       if (ret < 0)
> +               return ret;
> +
> +       ret = scpsys_sram_disable(pd);
> +       if (ret < 0)
> +               return ret;
> +
> +       /* subsys power off */
> +       regmap_read(scpsys->base, pd->data->ctl_offs, &val);
> +       val |= PWR_ISO_BIT;
> +       regmap_write(scpsys->base, pd->data->ctl_offs, val);
> +
> +       val &= ~PWR_RST_B_BIT;
> +       regmap_write(scpsys->base, pd->data->ctl_offs, val);
> +
> +       val |= PWR_CLK_DIS_BIT;
> +       regmap_write(scpsys->base, pd->data->ctl_offs, val);
> +
> +       val &= ~PWR_ON_BIT;
> +       regmap_write(scpsys->base, pd->data->ctl_offs, val);
> +
> +       val &= ~PWR_ON_2ND_BIT;
> +       regmap_write(scpsys->base, pd->data->ctl_offs, val);
> +
> +       /* 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_probe(scpsys->dev, ret,
> +                             "%pOFn: failed to retrieve domain id from reg\n", node);
> +               return -EINVAL;
> +       }
> +
> +       if (id >= scpsys->soc_data->num_domains) {
> +               dev_err_probe(scpsys->dev, -EINVAL, "%pOFn: invalid domain id %d\n", node, id);
> +               return -EINVAL;
> +       }
> +
> +       domain_data = &scpsys->soc_data->domains[id];
> +       if (!domain_data) {

Is that even possible at all? I mean, even if
scpsys->soc_data->domains is NULL, as long as id != 0, this will no
happen.

> +               dev_err_probe(scpsys->dev, -EINVAL, "%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;
> +       }

pd->num_clks < 0 can't happen so you can drop this.

> +
> +       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_probe(scpsys->dev, ret, "%pOFn: failed to get clk at index %d\n",
> +                                     node, i);
> +                       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_probe(scpsys->dev, ret, "failed to power on domain %pOFN\n", node);
> +               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;

Do we want to add some logic to make sure scpsys->domains[id] == NULL
before this? (to make sure we don't accidentally probe 2 domains with
the same id, which I guess could happen with some incorrect
devicetree)

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

Why do you need these 2 lines? Can't you just let the core free the
devm_ allocated data?

> +       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_probe(scpsys->dev, ret,
> +                                     "%pOFn: failed to get parent domain id\n", child);
> +                       goto err_put_node;
> +               }
> +               parent_pd = scpsys->pd_data.domains[id];

No bounds check on id here? Also, wondering if you want to move the
logic, including bounds check, to a small function (since it's
repeated 3 times now, but I think that can be reduced to 2).

> +
> +               ret = scpsys_add_one_domain(scpsys, child);
> +               if (ret)
> +                       goto err_put_node;
> +
> +               ret = of_property_read_u32(child, "reg", &id);
> +               if (ret) {
> +                       dev_err_probe(scpsys->dev, ret,
> +                                     "%pOFn: failed to get child domain id\n", child);
> +                       goto err_put_node;
> +               }
> +               child_pd = scpsys->pd_data.domains[id];

You already obtained the child_pd in scpsys_add_one_domain... Can you
have scpsys_add_one_domain return `child_pd` as a pointer (or a
PTR_ERR)?

> +
> +               ret = pm_genpd_add_subdomain(parent_pd, child_pd);
> +               if (ret) {
> +                       dev_err_probe(scpsys->dev, ret,
> +                                     "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;

What's the point of setting num_clks to 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);
> +               }
> +       }
> +}
> [snip]

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

* Re: [PATCH v2 06/12] soc: mediatek: pm-domains: Add SMI block as bus protection block
  2020-10-02  8:56   ` Matthias Brugger
@ 2020-10-05  1:48     ` Nicolas Boichat
  2020-10-05 10:28       ` Matthias Brugger
  2020-10-26 15:18       ` Enric Balletbo i Serra
  2020-10-26 15:18     ` Enric Balletbo i Serra
  1 sibling, 2 replies; 30+ messages in thread
From: Nicolas Boichat @ 2020-10-05  1:48 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Enric Balletbo i Serra, lkml, Collabora Kernel ML, Fabien Parent,
	Hsin-Yi Wang, Weiyi Lu, Matthias Brugger, linux-arm Mailing List,
	moderated list:ARM/Mediatek SoC support

On Fri, Oct 2, 2020 at 4:56 PM Matthias Brugger <matthias.bgg@gmail.com> wrote:
>
>
>
> On 01/10/2020 18:01, 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>
> > ---
> >
> > Changes in v2: None
> >
> >   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 b5e7c9846c34..38f2630bdd0a 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 {
> > @@ -175,9 +195,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)
> > @@ -193,7 +213,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);

Not a huge fan or reusing bpd for 2 different things.

I think this is easier to follow:

_scpsys_bus_protect_enable(pd->data->bp_infracfg, pd->infracfg);
...
_scpsys_bus_protect_enable(pd->data->bp_smi, pd->smi);


> >   }
> >  [snip]

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

* Re: [PATCH v2 06/12] soc: mediatek: pm-domains: Add SMI block as bus protection block
  2020-10-05  1:48     ` Nicolas Boichat
@ 2020-10-05 10:28       ` Matthias Brugger
  2020-10-26 15:18       ` Enric Balletbo i Serra
  1 sibling, 0 replies; 30+ messages in thread
From: Matthias Brugger @ 2020-10-05 10:28 UTC (permalink / raw)
  To: Nicolas Boichat, Matthias Brugger
  Cc: Enric Balletbo i Serra, lkml, Collabora Kernel ML, Fabien Parent,
	Hsin-Yi Wang, Weiyi Lu, linux-arm Mailing List,
	moderated list:ARM/Mediatek SoC support



On 05/10/2020 03:48, Nicolas Boichat wrote:
> On Fri, Oct 2, 2020 at 4:56 PM Matthias Brugger <matthias.bgg@gmail.com> wrote:
>>
>>
>>
>> On 01/10/2020 18:01, 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>
>>> ---
>>>
>>> Changes in v2: None
>>>
>>>    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 b5e7c9846c34..38f2630bdd0a 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 {
>>> @@ -175,9 +195,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)
>>> @@ -193,7 +213,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);
> 
> Not a huge fan or reusing bpd for 2 different things.
> 
> I think this is easier to follow:
> 
> _scpsys_bus_protect_enable(pd->data->bp_infracfg, pd->infracfg);
> ...
> _scpsys_bus_protect_enable(pd->data->bp_smi, pd->smi);
> 

Sounds reasonable, yes :)

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

* Re: [PATCH v2 12/12] arm64: dts: mediatek: Add mt8183 power domains controller
  2020-10-01 16:01 ` [PATCH v2 12/12] arm64: dts: mediatek: Add mt8183 power domains controller Enric Balletbo i Serra
  2020-10-02  9:11   ` Matthias Brugger
@ 2020-10-11 14:55   ` kernel test robot
  1 sibling, 0 replies; 30+ messages in thread
From: kernel test robot @ 2020-10-11 14:55 UTC (permalink / raw)
  To: Enric Balletbo i Serra, linux-kernel
  Cc: kbuild-all, Collabora Kernel ML, fparent, matthias.bgg, drinkcat,
	hsinyi, weiyi.lu, Matthias Brugger, Rob Herring, devicetree

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

Hi Enric,

I love your patch! Yet something to improve:

[auto build test ERROR on robh/for-next]
[also build test ERROR on mediatek/for-next linus/master v5.9-rc8]
[cannot apply to next-20201009]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Enric-Balletbo-i-Serra/soc-mediatek-pm-domains-Add-new-driver-for-SCPSYS-power-domains-controller/20201002-000415
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/676cab78e14a3238d1d3fa2a601058eb3ac38a03
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Enric-Balletbo-i-Serra/soc-mediatek-pm-domains-Add-new-driver-for-SCPSYS-power-domains-controller/20201002-000415
        git checkout 676cab78e14a3238d1d3fa2a601058eb3ac38a03
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> ERROR: Input tree has errors, aborting (use -f to force output)

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 51977 bytes --]

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

* Re: [PATCH v2 02/12] soc: mediatek: Add MediaTek SCPSYS power domains
  2020-10-05  1:39   ` Nicolas Boichat
@ 2020-10-26 15:17     ` Enric Balletbo i Serra
  2020-10-27  0:19       ` Nicolas Boichat
  0 siblings, 1 reply; 30+ messages in thread
From: Enric Balletbo i Serra @ 2020-10-26 15:17 UTC (permalink / raw)
  To: Nicolas Boichat
  Cc: lkml, Collabora Kernel ML, Fabien Parent, Matthias Brugger,
	Hsin-Yi Wang, Weiyi Lu, Matthias Brugger, linux-arm Mailing List,
	moderated list:ARM/Mediatek SoC support

Hi Nicolas,

Many thanks for looking at this.

On 5/10/20 3:39, Nicolas Boichat wrote:
> On Fri, Oct 2, 2020 at 12:02 AM Enric Balletbo i Serra
> <enric.balletbo@collabora.com> 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>
>> ---
>>
>> Changes in v2:
>> - Get base address from parent syscon. We have now a scpsys syscon node
>>   and a child for the SPM (System Power Manager).
>> - Use regmap API to acces de base address.
>>
>>  drivers/soc/mediatek/Kconfig          |  13 +
>>  drivers/soc/mediatek/Makefile         |   1 +
>>  drivers/soc/mediatek/mtk-pm-domains.c | 628 ++++++++++++++++++++++++++
>>  3 files changed, 642 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..68886bf437f9
>> --- /dev/null
>> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
>> @@ -0,0 +1,628 @@
>> [snip]
>> +
>> +static int scpsys_domain_is_on(struct scpsys_domain *pd)
>> +{
>> +       struct scpsys *scpsys = pd->scpsys;
>> +       u32 status, status2;
>> +
>> +       regmap_read(scpsys->base, scpsys->soc_data->pwr_sta_offs, &status);
>> +       status &= pd->data->sta_mask;
>> +
>> +       regmap_read(scpsys->base, scpsys->soc_data->pwr_sta2nd_offs, &status2);
>> +       status2 &= 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;
> 
> 
> Why do you care? It seems like all you ever do with this is check if
> the domain is powered on, so it seems like you could just
> return (status && status2); ?
> 

Changed in v3


>> +}
>> +
>> +static int scpsys_sram_enable(struct scpsys_domain *pd)
>> +{
>> +       u32 pdn_ack = pd->data->sram_pdn_ack_bits;
>> +       struct scpsys *scpsys = pd->scpsys;
>> +       u32 val;
>> +       int tmp;
> 
> regmap_read_poll_timeout => regmap_read technically takes a `unsigned
> int *` parameter, so this should probably be unsigned too (or... just
> reuse val?)
> 

Changed in v3

>> +
>> +       regmap_read(scpsys->base, pd->data->ctl_offs, &val);
>> +       val &= ~pd->data->sram_pdn_bits;
>> +       regmap_write(scpsys->base, pd->data->ctl_offs, val);
> 
> Replace with regmap_update_bits?
> 

Right, I'll replace here and in the other places where is possible.

>> +
>> +       /* Either wait until SRAM_PDN_ACK all 1 or 0 */
>> +       return regmap_read_poll_timeout(scpsys->base, pd->data->ctl_offs, tmp,
>> +                                       (tmp & pdn_ack) == 0, MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
>> +}
>> +
>> +static int scpsys_sram_disable(struct scpsys_domain *pd)
>> +{
>> +       u32 pdn_ack = pd->data->sram_pdn_ack_bits;
>> +       struct scpsys *scpsys = pd->scpsys;
>> +       u32 val;
>> +       int tmp;
>> +
>> +       regmap_read(scpsys->base, pd->data->ctl_offs, &val);
>> +       val |= pd->data->sram_pdn_bits;
>> +       regmap_write(scpsys->base, pd->data->ctl_offs, val);
>> +
>> +       /* Either wait until SRAM_PDN_ACK all 1 or 0 */
>> +       return regmap_read_poll_timeout(scpsys->base, pd->data->ctl_offs, 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;
>> +       int ret, tmp;
>> +       u32 val;
>> +
>> +       ret = clk_bulk_enable(pd->num_clks, pd->clks);
>> +       if (ret)
>> +               return ret;
>> +
>> +       /* subsys power on */
>> +       regmap_read(scpsys->base, pd->data->ctl_offs, &val);
>> +       val |= PWR_ON_BIT;
>> +       regmap_write(scpsys->base, pd->data->ctl_offs, val);
>> +       val |= PWR_ON_2ND_BIT;
>> +       regmap_write(scpsys->base, pd->data->ctl_offs, val);
>> +
>> +       /* 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;
>> +       regmap_write(scpsys->base, pd->data->ctl_offs, val);
>> +
>> +       val &= ~PWR_ISO_BIT;
>> +       regmap_write(scpsys->base, pd->data->ctl_offs, val);
>> +
>> +       val |= PWR_RST_B_BIT;
>> +       regmap_write(scpsys->base, pd->data->ctl_offs, val);
>> +
>> +       ret = scpsys_sram_enable(pd);
>> +       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);
>> +       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;
>> +       int ret, tmp;
>> +       u32 val;
>> +
>> +       ret = scpsys_bus_protect_enable(pd);
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       ret = scpsys_sram_disable(pd);
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       /* subsys power off */
>> +       regmap_read(scpsys->base, pd->data->ctl_offs, &val);
>> +       val |= PWR_ISO_BIT;
>> +       regmap_write(scpsys->base, pd->data->ctl_offs, val);
>> +
>> +       val &= ~PWR_RST_B_BIT;
>> +       regmap_write(scpsys->base, pd->data->ctl_offs, val);
>> +
>> +       val |= PWR_CLK_DIS_BIT;
>> +       regmap_write(scpsys->base, pd->data->ctl_offs, val);
>> +
>> +       val &= ~PWR_ON_BIT;
>> +       regmap_write(scpsys->base, pd->data->ctl_offs, val);
>> +
>> +       val &= ~PWR_ON_2ND_BIT;
>> +       regmap_write(scpsys->base, pd->data->ctl_offs, val);
>> +
>> +       /* 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_probe(scpsys->dev, ret,
>> +                             "%pOFn: failed to retrieve domain id from reg\n", node);
>> +               return -EINVAL;
>> +       }
>> +
>> +       if (id >= scpsys->soc_data->num_domains) {
>> +               dev_err_probe(scpsys->dev, -EINVAL, "%pOFn: invalid domain id %d\n", node, id);
>> +               return -EINVAL;
>> +       }
>> +
>> +       domain_data = &scpsys->soc_data->domains[id];
>> +       if (!domain_data) {
> 
> Is that even possible at all? I mean, even if
> scpsys->soc_data->domains is NULL, as long as id != 0, this will no
> happen.
> 

I think could happen with a bad DT definition. I.e if for the definition of the
MT8173 domains you use a wrong value for the reg property, a value that is not
present in the SoC data. It is unlikely if you use the defines but could happen
if you hardcore the value. We cannot check this with the DT json-schema.

>> +               dev_err_probe(scpsys->dev, -EINVAL, "%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;
>> +       }
> 
> pd->num_clks < 0 can't happen so you can drop this.
> 

Right, removed in the next version.

>> +
>> +       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_probe(scpsys->dev, ret, "%pOFn: failed to get clk at index %d\n",
>> +                                     node, i);
>> +                       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_probe(scpsys->dev, ret, "failed to power on domain %pOFN\n", node);
>> +               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;
> 
> Do we want to add some logic to make sure scpsys->domains[id] == NULL
> before this? (to make sure we don't accidentally probe 2 domains with
> the same id, which I guess could happen with some incorrect
> devicetree)
> 

Good idea, I'll do in next version.



>> +       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;
> 
> Why do you need these 2 lines? Can't you just let the core free the
> devm_ allocated data?
> 

Right, this is in the probe path, so no need to free the resources, the core
will do it.

>> +       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_probe(scpsys->dev, ret,
>> +                                     "%pOFn: failed to get parent domain id\n", child);
>> +                       goto err_put_node;
>> +               }
>> +               parent_pd = scpsys->pd_data.domains[id];
> 
> No bounds check on id here? Also, wondering if you want to move the
> logic, including bounds check, to a small function (since it's
> repeated 3 times now, but I think that can be reduced to 2).
> 

Done in the next version.

>> +
>> +               ret = scpsys_add_one_domain(scpsys, child);
>> +               if (ret)
>> +                       goto err_put_node;
>> +
>> +               ret = of_property_read_u32(child, "reg", &id);
>> +               if (ret) {
>> +                       dev_err_probe(scpsys->dev, ret,
>> +                                     "%pOFn: failed to get child domain id\n", child);
>> +                       goto err_put_node;
>> +               }
>> +               child_pd = scpsys->pd_data.domains[id];
> 
> You already obtained the child_pd in scpsys_add_one_domain... Can you
> have scpsys_add_one_domain return `child_pd` as a pointer (or a
> PTR_ERR)?
> 

Good idea, done in next version.


>> +
>> +               ret = pm_genpd_add_subdomain(parent_pd, child_pd);
>> +               if (ret) {
>> +                       dev_err_probe(scpsys->dev, ret,
>> +                                     "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;
> 
> What's the point of setting num_clks to 0?
> 

No really needed, no.

>> +}
>> +
>> +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);
>> +               }
>> +       }
>> +}
>> [snip]

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

* Re: [PATCH v2 08/12] soc: mediatek: pm-domains: Add subsystem clocks
  2020-10-02  9:04   ` Matthias Brugger
@ 2020-10-26 15:17     ` Enric Balletbo i Serra
  0 siblings, 0 replies; 30+ messages in thread
From: Enric Balletbo i Serra @ 2020-10-26 15:17 UTC (permalink / raw)
  To: Matthias Brugger, linux-kernel
  Cc: Collabora Kernel ML, fparent, drinkcat, hsinyi, weiyi.lu,
	Matthias Brugger, linux-arm-kernel, linux-mediatek

Hi Matthias,

Thank you for your review.

On 2/10/20 11:04, Matthias Brugger wrote:
> 
> 
> On 01/10/2020 18:01, 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>
>> ---
>>
>> Changes in v2:
>> - Use dev_err_probe if getting clocks fails, so an error is not printed
>>    if is deferred.
>>
>>   drivers/soc/mediatek/mtk-pm-domains.c | 89 ++++++++++++++++++++++-----
>>   1 file changed, 75 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c
>> b/drivers/soc/mediatek/mtk-pm-domains.c
>> index e0a52d489fea..2075072f16da 100644
>> --- a/drivers/soc/mediatek/mtk-pm-domains.c
>> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> [...]
>>   +    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_probe(scpsys->dev, PTR_ERR(clk),
>> +                      "%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;
>> +
> 
> Not sure if it really matters, but logically we should prepare the basic clocks
> before we prepare the subsystem clocks. And fix the error handling accordingly.
> 

Changed in next version.

> Regards,
> Matthias
> 
>>       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
>> @@ -462,6 +513,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);
>> @@ -541,6 +598,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)
>>

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

* Re: [PATCH v2 06/12] soc: mediatek: pm-domains: Add SMI block as bus protection block
  2020-10-02  8:56   ` Matthias Brugger
  2020-10-05  1:48     ` Nicolas Boichat
@ 2020-10-26 15:18     ` Enric Balletbo i Serra
  1 sibling, 0 replies; 30+ messages in thread
From: Enric Balletbo i Serra @ 2020-10-26 15:18 UTC (permalink / raw)
  To: Matthias Brugger, linux-kernel
  Cc: Collabora Kernel ML, fparent, drinkcat, hsinyi, weiyi.lu,
	Matthias Brugger, linux-arm-kernel, linux-mediatek

Hi Matthias,

Thank you for your review.

On 2/10/20 10:56, Matthias Brugger wrote:
> 
> 
> On 01/10/2020 18:01, 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>
>> ---
>>
>> Changes in v2: None
>>
>>   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 b5e7c9846c34..38f2630bdd0a 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 {
>> @@ -175,9 +195,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)
>> @@ -193,7 +213,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,
>> @@ -208,11 +232,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)
>> @@ -228,7 +252,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);
> 
> Also it seems not to break the system we should disable pd->smi first and after
> that pd->infracfg (just the other way round as we did in enable).
> 

Done in next version.


>>   }
>>     static int scpsys_power_on(struct generic_pm_domain *genpd)
>> @@ -358,6 +386,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);
>> @@ -528,10 +560,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] = {
>> @@ -570,12 +601,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)            \
> 
> While at it, could you update this to call it:
> BUS_PROT_UPDATE_TOPAXI as in all the other SoCs TOPAXI is mapped to the same
> address.
> 

Sure, done in next version.

> Thanks!
> Matthias
> 
>> +    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,
>>

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

* Re: [PATCH v2 06/12] soc: mediatek: pm-domains: Add SMI block as bus protection block
  2020-10-05  1:48     ` Nicolas Boichat
  2020-10-05 10:28       ` Matthias Brugger
@ 2020-10-26 15:18       ` Enric Balletbo i Serra
  1 sibling, 0 replies; 30+ messages in thread
From: Enric Balletbo i Serra @ 2020-10-26 15:18 UTC (permalink / raw)
  To: Nicolas Boichat, Matthias Brugger
  Cc: lkml, Collabora Kernel ML, Fabien Parent, Hsin-Yi Wang, Weiyi Lu,
	Matthias Brugger, linux-arm Mailing List,
	moderated list:ARM/Mediatek SoC support

Hi Nicolas,

Thank you for your review.

On 5/10/20 3:48, Nicolas Boichat wrote:
> On Fri, Oct 2, 2020 at 4:56 PM Matthias Brugger <matthias.bgg@gmail.com> wrote:
>>
>>
>>
>> On 01/10/2020 18:01, 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>
>>> ---
>>>
>>> Changes in v2: None
>>>
>>>   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 b5e7c9846c34..38f2630bdd0a 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 {
>>> @@ -175,9 +195,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)
>>> @@ -193,7 +213,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);
> 
> Not a huge fan or reusing bpd for 2 different things.
> 
> I think this is easier to follow:
> 
> _scpsys_bus_protect_enable(pd->data->bp_infracfg, pd->infracfg);
> ...
> _scpsys_bus_protect_enable(pd->data->bp_smi, pd->smi);
> 

Makes sense, applied in next version.

> 
>>>   }
>>>  [snip]

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

* Re: [PATCH v2 02/12] soc: mediatek: Add MediaTek SCPSYS power domains
  2020-10-26 15:17     ` Enric Balletbo i Serra
@ 2020-10-27  0:19       ` Nicolas Boichat
  2020-10-27 16:25         ` Enric Balletbo i Serra
  0 siblings, 1 reply; 30+ messages in thread
From: Nicolas Boichat @ 2020-10-27  0:19 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: lkml, Collabora Kernel ML, Fabien Parent, Matthias Brugger,
	Hsin-Yi Wang, Weiyi Lu, Matthias Brugger, linux-arm Mailing List,
	moderated list:ARM/Mediatek SoC support

Hi Enric,

On Mon, Oct 26, 2020 at 11:17 PM Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
>
> Hi Nicolas,
>
> Many thanks for looking at this.

Thanks to you ,-)

[snip]
> >> +       if (id >= scpsys->soc_data->num_domains) {
> >> +               dev_err_probe(scpsys->dev, -EINVAL, "%pOFn: invalid domain id %d\n", node, id);
> >> +               return -EINVAL;
> >> +       }
> >> +
> >> +       domain_data = &scpsys->soc_data->domains[id];
> >> +       if (!domain_data) {
> >
> > Is that even possible at all? I mean, even if
> > scpsys->soc_data->domains is NULL, as long as id != 0, this will no
> > happen.
> >
>
> I think could happen with a bad DT definition. I.e if for the definition of the
> MT8173 domains you use a wrong value for the reg property, a value that is not
> present in the SoC data. It is unlikely if you use the defines but could happen
> if you hardcore the value. We cannot check this with the DT json-schema.

I wasn't clear in my explanation, and looking further there is more
that looks wrong.

This expression &scpsys->soc_data->domains[id] is a pointer to element
"id" of the array domains. So if you convert to integer arithmetic,
it'll be something like `(long)scpsys->soc_data->domains +
(sizeof(struct generic_pm_domain *)) * id`. The only way this can be
NULL is if scpsys->soc_data->domains pointer is NULL, which, actually,
can't really happen as it's the 5th element of a struct scpsys
structure `(long)scpsys->soc_data + offset_of(domains, struct scpsys)
+ (sizeof(struct generic_pm_domain *)) * id`.

I think what you mean is either:
domain_data = &scpsys->soc_data->domains[id];
if (!*domain_data)
[but then domain_data type should be `struct generic_pm_domain **`?
Does your code compile with warnings enabled?]
or:
domain_data = scpsys->soc_data->domains[id];
if (!domain_data)
[then the test makes sense]

[snip]

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

* Re: [PATCH v2 02/12] soc: mediatek: Add MediaTek SCPSYS power domains
  2020-10-27  0:19       ` Nicolas Boichat
@ 2020-10-27 16:25         ` Enric Balletbo i Serra
  2020-10-28  1:13           ` Nicolas Boichat
  0 siblings, 1 reply; 30+ messages in thread
From: Enric Balletbo i Serra @ 2020-10-27 16:25 UTC (permalink / raw)
  To: Nicolas Boichat
  Cc: lkml, Collabora Kernel ML, Fabien Parent, Matthias Brugger,
	Hsin-Yi Wang, Weiyi Lu, Matthias Brugger, linux-arm Mailing List,
	moderated list:ARM/Mediatek SoC support

Hi Nicolas,

On 27/10/20 1:19, Nicolas Boichat wrote:
> Hi Enric,
> 
> On Mon, Oct 26, 2020 at 11:17 PM Enric Balletbo i Serra
> <enric.balletbo@collabora.com> wrote:
>>
>> Hi Nicolas,
>>
>> Many thanks for looking at this.
> 
> Thanks to you ,-)
> 
> [snip]
>>>> +       if (id >= scpsys->soc_data->num_domains) {
>>>> +               dev_err_probe(scpsys->dev, -EINVAL, "%pOFn: invalid domain id %d\n", node, id);
>>>> +               return -EINVAL;
>>>> +       }
>>>> +
>>>> +       domain_data = &scpsys->soc_data->domains[id];
>>>> +       if (!domain_data) {
>>>
>>> Is that even possible at all? I mean, even if
>>> scpsys->soc_data->domains is NULL, as long as id != 0, this will no
>>> happen.
>>>
>>
>> I think could happen with a bad DT definition. I.e if for the definition of the
>> MT8173 domains you use a wrong value for the reg property, a value that is not
>> present in the SoC data. It is unlikely if you use the defines but could happen
>> if you hardcore the value. We cannot check this with the DT json-schema.
> 
> I wasn't clear in my explanation, and looking further there is more
> that looks wrong.
> 
> This expression &scpsys->soc_data->domains[id] is a pointer to element
> "id" of the array domains. So if you convert to integer arithmetic,
> it'll be something like `(long)scpsys->soc_data->domains +
> (sizeof(struct generic_pm_domain *)) * id`. The only way this can be
> NULL is if scpsys->soc_data->domains pointer is NULL, which, actually,
> can't really happen as it's the 5th element of a struct scpsys
> structure `(long)scpsys->soc_data + offset_of(domains, struct scpsys)
> + (sizeof(struct generic_pm_domain *)) * id`.
> 
> I think what you mean is either:
> domain_data = &scpsys->soc_data->domains[id];
> if (!*domain_data)
> [but then domain_data type should be `struct generic_pm_domain **`?

I think you're confusing the field `struct generic_pm_domain *domains[]`from the
`struct scpsys` with `const struct scpsys_domain_data *domains` from `struct
scpsys_soc_data`. My bad they have the same name, I should probably rename the
second one as domain_info or domain_data to avoid that confusion.


diff --git a/drivers/soc/mediatek/mtk-pm-domains.h
b/drivers/soc/mediatek/mtk-pm-domains.h
index 7c8efcb3cef2..6ff095db8a27 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.h
+++ b/drivers/soc/mediatek/mtk-pm-domains.h
@@ -56,7 +56,7 @@ struct scpsys_domain_data {
 };

 struct scpsys_soc_data {
-       const struct scpsys_domain_data *domains;
+       const struct scpsys_domain_data *domain_data;
        int num_domains;
        int pwr_sta_offs;
        int pwr_sta2nd_offs;

---

struct scpsys {
    ...
    const struct scpsys_soc_data *soc_data;
    ...
    struct generic_pm_domain *domains[];
}


domain_data = &scpsys->soc_data->domain_data[id];
if (!domain_data)

Thanks,
  Enric


> Does your code compile with warnings enabled?]
> or:
> domain_data = scpsys->soc_data->domains[id];
> if (!domain_data)
> [then the test makes sense]
> 
> [snip]
> 

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

* Re: [PATCH v2 02/12] soc: mediatek: Add MediaTek SCPSYS power domains
  2020-10-27 16:25         ` Enric Balletbo i Serra
@ 2020-10-28  1:13           ` Nicolas Boichat
  2020-10-30 10:29             ` Enric Balletbo i Serra
  0 siblings, 1 reply; 30+ messages in thread
From: Nicolas Boichat @ 2020-10-28  1:13 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: lkml, Collabora Kernel ML, Fabien Parent, Matthias Brugger,
	Hsin-Yi Wang, Weiyi Lu, Matthias Brugger, linux-arm Mailing List,
	moderated list:ARM/Mediatek SoC support

On Wed, Oct 28, 2020 at 12:25 AM Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
>
> Hi Nicolas,
>
> On 27/10/20 1:19, Nicolas Boichat wrote:
> > Hi Enric,
> >
> > On Mon, Oct 26, 2020 at 11:17 PM Enric Balletbo i Serra
> > <enric.balletbo@collabora.com> wrote:
> >>
> >> Hi Nicolas,
> >>
> >> Many thanks for looking at this.
> >
> > Thanks to you ,-)
> >
> > [snip]
> >>>> +       if (id >= scpsys->soc_data->num_domains) {
> >>>> +               dev_err_probe(scpsys->dev, -EINVAL, "%pOFn: invalid domain id %d\n", node, id);
> >>>> +               return -EINVAL;
> >>>> +       }
> >>>> +
> >>>> +       domain_data = &scpsys->soc_data->domains[id];
> >>>> +       if (!domain_data) {
> >>>
> >>> Is that even possible at all? I mean, even if
> >>> scpsys->soc_data->domains is NULL, as long as id != 0, this will no
> >>> happen.
> >>>
> >>
> >> I think could happen with a bad DT definition. I.e if for the definition of the
> >> MT8173 domains you use a wrong value for the reg property, a value that is not
> >> present in the SoC data. It is unlikely if you use the defines but could happen
> >> if you hardcore the value. We cannot check this with the DT json-schema.
> >
> > I wasn't clear in my explanation, and looking further there is more
> > that looks wrong.
> >
> > This expression &scpsys->soc_data->domains[id] is a pointer to element
> > "id" of the array domains. So if you convert to integer arithmetic,
> > it'll be something like `(long)scpsys->soc_data->domains +
> > (sizeof(struct generic_pm_domain *)) * id`. The only way this can be
> > NULL is if scpsys->soc_data->domains pointer is NULL, which, actually,
> > can't really happen as it's the 5th element of a struct scpsys
> > structure `(long)scpsys->soc_data + offset_of(domains, struct scpsys)
> > + (sizeof(struct generic_pm_domain *)) * id`.
> >
> > I think what you mean is either:
> > domain_data = &scpsys->soc_data->domains[id];
> > if (!*domain_data)
> > [but then domain_data type should be `struct generic_pm_domain **`?
>
> I think you're confusing the field `struct generic_pm_domain *domains[]`from the
> `struct scpsys` with `const struct scpsys_domain_data *domains` from `struct
> scpsys_soc_data`. My bad they have the same name, I should probably rename the
> second one as domain_info or domain_data to avoid that confusion.

Oh, okay, get it, thanks for clarifying, I got myself confused indeed ,-P

But, still, part of my integer arithmetics still holds...

&scpsys->soc_data->domains[id] = (long)scpsys->soc_data->domains +
(sizeof(struct generic_pm_domain *)) * id. The only way domain_data
can be NULL is if scpsys->soc_data->domains pointer is NULL (it can't
be, really, assuming scpsys_soc_data structures are well defined) AND
id is 0.

Now, if I understand what you want to check here. If a domain id is
not specified in scpsys_domain_data (e.g. if there is a gap in
MT8XXX_POWER_DOMAIN_YYY indices and if `id` points at one of those
gaps), you'll get an all-zero entry in domain_data. So maybe you can
just check that domain_data->sta_mask != 0? Would that be enough? (I
expect that sta_mask would always need to be set?)

But then again, are there ever gaps in MT8XXX_POWER_DOMAIN_YYY indices?

>
>
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.h
> b/drivers/soc/mediatek/mtk-pm-domains.h
> index 7c8efcb3cef2..6ff095db8a27 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.h
> +++ b/drivers/soc/mediatek/mtk-pm-domains.h
> @@ -56,7 +56,7 @@ struct scpsys_domain_data {
>  };
>
>  struct scpsys_soc_data {
> -       const struct scpsys_domain_data *domains;
> +       const struct scpsys_domain_data *domain_data;
>         int num_domains;
>         int pwr_sta_offs;
>         int pwr_sta2nd_offs;
>
> ---
>
> struct scpsys {
>     ...
>     const struct scpsys_soc_data *soc_data;
>     ...
>     struct generic_pm_domain *domains[];
> }
>
>
> domain_data = &scpsys->soc_data->domain_data[id];
> if (!domain_data)
>
> Thanks,
>   Enric
>
>
> > Does your code compile with warnings enabled?]
> > or:
> > domain_data = scpsys->soc_data->domains[id];
> > if (!domain_data)
> > [then the test makes sense]
> >
> > [snip]
> >

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

* Re: [PATCH v2 02/12] soc: mediatek: Add MediaTek SCPSYS power domains
  2020-10-28  1:13           ` Nicolas Boichat
@ 2020-10-30 10:29             ` Enric Balletbo i Serra
  2020-10-30 12:44               ` Nicolas Boichat
  0 siblings, 1 reply; 30+ messages in thread
From: Enric Balletbo i Serra @ 2020-10-30 10:29 UTC (permalink / raw)
  To: Nicolas Boichat
  Cc: lkml, Collabora Kernel ML, Fabien Parent, Matthias Brugger,
	Hsin-Yi Wang, Weiyi Lu, Matthias Brugger, linux-arm Mailing List,
	moderated list:ARM/Mediatek SoC support

Hi Nicolas,

On 28/10/20 2:13, Nicolas Boichat wrote:
> On Wed, Oct 28, 2020 at 12:25 AM Enric Balletbo i Serra
> <enric.balletbo@collabora.com> wrote:
>>
>> Hi Nicolas,
>>
>> On 27/10/20 1:19, Nicolas Boichat wrote:
>>> Hi Enric,
>>>
>>> On Mon, Oct 26, 2020 at 11:17 PM Enric Balletbo i Serra
>>> <enric.balletbo@collabora.com> wrote:
>>>>
>>>> Hi Nicolas,
>>>>
>>>> Many thanks for looking at this.
>>>
>>> Thanks to you ,-)
>>>
>>> [snip]
>>>>>> +       if (id >= scpsys->soc_data->num_domains) {
>>>>>> +               dev_err_probe(scpsys->dev, -EINVAL, "%pOFn: invalid domain id %d\n", node, id);
>>>>>> +               return -EINVAL;
>>>>>> +       }
>>>>>> +
>>>>>> +       domain_data = &scpsys->soc_data->domains[id];
>>>>>> +       if (!domain_data) {
>>>>>
>>>>> Is that even possible at all? I mean, even if
>>>>> scpsys->soc_data->domains is NULL, as long as id != 0, this will no
>>>>> happen.
>>>>>
>>>>
>>>> I think could happen with a bad DT definition. I.e if for the definition of the
>>>> MT8173 domains you use a wrong value for the reg property, a value that is not
>>>> present in the SoC data. It is unlikely if you use the defines but could happen
>>>> if you hardcore the value. We cannot check this with the DT json-schema.
>>>
>>> I wasn't clear in my explanation, and looking further there is more
>>> that looks wrong.
>>>
>>> This expression &scpsys->soc_data->domains[id] is a pointer to element
>>> "id" of the array domains. So if you convert to integer arithmetic,
>>> it'll be something like `(long)scpsys->soc_data->domains +
>>> (sizeof(struct generic_pm_domain *)) * id`. The only way this can be
>>> NULL is if scpsys->soc_data->domains pointer is NULL, which, actually,
>>> can't really happen as it's the 5th element of a struct scpsys
>>> structure `(long)scpsys->soc_data + offset_of(domains, struct scpsys)
>>> + (sizeof(struct generic_pm_domain *)) * id`.
>>>
>>> I think what you mean is either:
>>> domain_data = &scpsys->soc_data->domains[id];
>>> if (!*domain_data)
>>> [but then domain_data type should be `struct generic_pm_domain **`?
>>
>> I think you're confusing the field `struct generic_pm_domain *domains[]`from the
>> `struct scpsys` with `const struct scpsys_domain_data *domains` from `struct
>> scpsys_soc_data`. My bad they have the same name, I should probably rename the
>> second one as domain_info or domain_data to avoid that confusion.
> 
> Oh, okay, get it, thanks for clarifying, I got myself confused indeed ,-P
> 
> But, still, part of my integer arithmetics still holds...
> 
> &scpsys->soc_data->domains[id] = (long)scpsys->soc_data->domains +
> (sizeof(struct generic_pm_domain *)) * id. The only way domain_data
> can be NULL is if scpsys->soc_data->domains pointer is NULL (it can't
> be, really, assuming scpsys_soc_data structures are well defined) AND
> id is 0.
> 
> Now, if I understand what you want to check here. If a domain id is
> not specified in scpsys_domain_data (e.g. if there is a gap in
> MT8XXX_POWER_DOMAIN_YYY indices and if `id` points at one of those
> gaps), you'll get an all-zero entry in domain_data. So maybe you can
> just check that domain_data->sta_mask != 0? Would that be enough? (I
> expect that sta_mask would always need to be set?)
> 

Yes, that would be enough. I'll change for the next version.

> But then again, are there ever gaps in MT8XXX_POWER_DOMAIN_YYY indices?
> 

AFAIK, there is no gaps, but one could make gaps when filling that info.  I
still think is worth have this check although is "unlikely" to happen due an
human error :-). I'll maintain for the next version, but I don't really care to
remove it if all you prefer I remove it.

Thanks,
  Enric


>>
>>
>> diff --git a/drivers/soc/mediatek/mtk-pm-domains.h
>> b/drivers/soc/mediatek/mtk-pm-domains.h
>> index 7c8efcb3cef2..6ff095db8a27 100644
>> --- a/drivers/soc/mediatek/mtk-pm-domains.h
>> +++ b/drivers/soc/mediatek/mtk-pm-domains.h
>> @@ -56,7 +56,7 @@ struct scpsys_domain_data {
>>  };
>>
>>  struct scpsys_soc_data {
>> -       const struct scpsys_domain_data *domains;
>> +       const struct scpsys_domain_data *domain_data;
>>         int num_domains;
>>         int pwr_sta_offs;
>>         int pwr_sta2nd_offs;
>>
>> ---
>>
>> struct scpsys {
>>     ...
>>     const struct scpsys_soc_data *soc_data;
>>     ...
>>     struct generic_pm_domain *domains[];
>> }
>>
>>
>> domain_data = &scpsys->soc_data->domain_data[id];
>> if (!domain_data)
>>
>> Thanks,
>>   Enric
>>
>>
>>> Does your code compile with warnings enabled?]
>>> or:
>>> domain_data = scpsys->soc_data->domains[id];
>>> if (!domain_data)
>>> [then the test makes sense]
>>>
>>> [snip]
>>>

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

* Re: [PATCH v2 02/12] soc: mediatek: Add MediaTek SCPSYS power domains
  2020-10-30 10:29             ` Enric Balletbo i Serra
@ 2020-10-30 12:44               ` Nicolas Boichat
  0 siblings, 0 replies; 30+ messages in thread
From: Nicolas Boichat @ 2020-10-30 12:44 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: lkml, Collabora Kernel ML, Fabien Parent, Matthias Brugger,
	Hsin-Yi Wang, Weiyi Lu, Matthias Brugger, linux-arm Mailing List,
	moderated list:ARM/Mediatek SoC support

On Fri, Oct 30, 2020 at 6:30 PM Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
>
> Hi Nicolas,
>
> On 28/10/20 2:13, Nicolas Boichat wrote:
> > On Wed, Oct 28, 2020 at 12:25 AM Enric Balletbo i Serra
> > <enric.balletbo@collabora.com> wrote:
> >>
> >> Hi Nicolas,
> >>
> >> On 27/10/20 1:19, Nicolas Boichat wrote:
> >>> Hi Enric,
> >>>
> >>> On Mon, Oct 26, 2020 at 11:17 PM Enric Balletbo i Serra
> >>> <enric.balletbo@collabora.com> wrote:
> >>>>
> >>>> Hi Nicolas,
> >>>>
> >>>> Many thanks for looking at this.
> >>>
> >>> Thanks to you ,-)
> >>>
> >>> [snip]
> >>>>>> +       if (id >= scpsys->soc_data->num_domains) {
> >>>>>> +               dev_err_probe(scpsys->dev, -EINVAL, "%pOFn: invalid domain id %d\n", node, id);
> >>>>>> +               return -EINVAL;
> >>>>>> +       }
> >>>>>> +
> >>>>>> +       domain_data = &scpsys->soc_data->domains[id];
> >>>>>> +       if (!domain_data) {
> >>>>>
> >>>>> Is that even possible at all? I mean, even if
> >>>>> scpsys->soc_data->domains is NULL, as long as id != 0, this will no
> >>>>> happen.
> >>>>>
> >>>>
> >>>> I think could happen with a bad DT definition. I.e if for the definition of the
> >>>> MT8173 domains you use a wrong value for the reg property, a value that is not
> >>>> present in the SoC data. It is unlikely if you use the defines but could happen
> >>>> if you hardcore the value. We cannot check this with the DT json-schema.
> >>>
> >>> I wasn't clear in my explanation, and looking further there is more
> >>> that looks wrong.
> >>>
> >>> This expression &scpsys->soc_data->domains[id] is a pointer to element
> >>> "id" of the array domains. So if you convert to integer arithmetic,
> >>> it'll be something like `(long)scpsys->soc_data->domains +
> >>> (sizeof(struct generic_pm_domain *)) * id`. The only way this can be
> >>> NULL is if scpsys->soc_data->domains pointer is NULL, which, actually,
> >>> can't really happen as it's the 5th element of a struct scpsys
> >>> structure `(long)scpsys->soc_data + offset_of(domains, struct scpsys)
> >>> + (sizeof(struct generic_pm_domain *)) * id`.
> >>>
> >>> I think what you mean is either:
> >>> domain_data = &scpsys->soc_data->domains[id];
> >>> if (!*domain_data)
> >>> [but then domain_data type should be `struct generic_pm_domain **`?
> >>
> >> I think you're confusing the field `struct generic_pm_domain *domains[]`from the
> >> `struct scpsys` with `const struct scpsys_domain_data *domains` from `struct
> >> scpsys_soc_data`. My bad they have the same name, I should probably rename the
> >> second one as domain_info or domain_data to avoid that confusion.
> >
> > Oh, okay, get it, thanks for clarifying, I got myself confused indeed ,-P
> >
> > But, still, part of my integer arithmetics still holds...
> >
> > &scpsys->soc_data->domains[id] = (long)scpsys->soc_data->domains +
> > (sizeof(struct generic_pm_domain *)) * id. The only way domain_data
> > can be NULL is if scpsys->soc_data->domains pointer is NULL (it can't
> > be, really, assuming scpsys_soc_data structures are well defined) AND
> > id is 0.
> >
> > Now, if I understand what you want to check here. If a domain id is
> > not specified in scpsys_domain_data (e.g. if there is a gap in
> > MT8XXX_POWER_DOMAIN_YYY indices and if `id` points at one of those
> > gaps), you'll get an all-zero entry in domain_data. So maybe you can
> > just check that domain_data->sta_mask != 0? Would that be enough? (I
> > expect that sta_mask would always need to be set?)
> >
>
> Yes, that would be enough. I'll change for the next version.
>
> > But then again, are there ever gaps in MT8XXX_POWER_DOMAIN_YYY indices?
> >
>
> AFAIK, there is no gaps, but one could make gaps when filling that info.  I
> still think is worth have this check although is "unlikely" to happen due an
> human error :-). I'll maintain for the next version, but I don't really care to
> remove it if all you prefer I remove it.

I'm fine with the sta_mask check. Thanks!

>
> Thanks,
>   Enric
>
>
> >>
> >>
> >> diff --git a/drivers/soc/mediatek/mtk-pm-domains.h
> >> b/drivers/soc/mediatek/mtk-pm-domains.h
> >> index 7c8efcb3cef2..6ff095db8a27 100644
> >> --- a/drivers/soc/mediatek/mtk-pm-domains.h
> >> +++ b/drivers/soc/mediatek/mtk-pm-domains.h
> >> @@ -56,7 +56,7 @@ struct scpsys_domain_data {
> >>  };
> >>
> >>  struct scpsys_soc_data {
> >> -       const struct scpsys_domain_data *domains;
> >> +       const struct scpsys_domain_data *domain_data;
> >>         int num_domains;
> >>         int pwr_sta_offs;
> >>         int pwr_sta2nd_offs;
> >>
> >> ---
> >>
> >> struct scpsys {
> >>     ...
> >>     const struct scpsys_soc_data *soc_data;
> >>     ...
> >>     struct generic_pm_domain *domains[];
> >> }
> >>
> >>
> >> domain_data = &scpsys->soc_data->domain_data[id];
> >> if (!domain_data)
> >>
> >> Thanks,
> >>   Enric
> >>
> >>
> >>> Does your code compile with warnings enabled?]
> >>> or:
> >>> domain_data = scpsys->soc_data->domains[id];
> >>> if (!domain_data)
> >>> [then the test makes sense]
> >>>
> >>> [snip]
> >>>

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

end of thread, other threads:[~2020-10-30 12:44 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-01 16:01 [PATCH v2 00/12] soc: mediatek: pm-domains: Add new driver for SCPSYS power domains controller Enric Balletbo i Serra
2020-10-01 16:01 ` [PATCH v2 01/12] dt-bindings: power: Add bindings for the Mediatek " Enric Balletbo i Serra
2020-10-01 16:01 ` [PATCH v2 02/12] soc: mediatek: Add MediaTek SCPSYS power domains Enric Balletbo i Serra
2020-10-05  1:39   ` Nicolas Boichat
2020-10-26 15:17     ` Enric Balletbo i Serra
2020-10-27  0:19       ` Nicolas Boichat
2020-10-27 16:25         ` Enric Balletbo i Serra
2020-10-28  1:13           ` Nicolas Boichat
2020-10-30 10:29             ` Enric Balletbo i Serra
2020-10-30 12:44               ` Nicolas Boichat
2020-10-01 16:01 ` [PATCH v2 03/12] arm64: dts: mediatek: Add mt8173 power domain controller Enric Balletbo i Serra
2020-10-01 16:01 ` [PATCH v2 04/12] soc: mediatek: pm-domains: Add bus protection protocol Enric Balletbo i Serra
2020-10-01 16:01 ` [PATCH v2 05/12] soc: mediatek: pm_domains: Make bus protection generic Enric Balletbo i Serra
2020-10-01 16:01 ` [PATCH v2 06/12] soc: mediatek: pm-domains: Add SMI block as bus protection block Enric Balletbo i Serra
2020-10-02  8:56   ` Matthias Brugger
2020-10-05  1:48     ` Nicolas Boichat
2020-10-05 10:28       ` Matthias Brugger
2020-10-26 15:18       ` Enric Balletbo i Serra
2020-10-26 15:18     ` Enric Balletbo i Serra
2020-10-01 16:01 ` [PATCH v2 07/12] soc: mediatek: pm-domains: Add extra sram control Enric Balletbo i Serra
2020-10-02  9:24   ` Matthias Brugger
2020-10-01 16:01 ` [PATCH v2 08/12] soc: mediatek: pm-domains: Add subsystem clocks Enric Balletbo i Serra
2020-10-02  9:04   ` Matthias Brugger
2020-10-26 15:17     ` Enric Balletbo i Serra
2020-10-01 16:01 ` [PATCH v2 09/12] soc: mediatek: pm-domains: Allow bus protection to ignore clear ack Enric Balletbo i Serra
2020-10-01 16:01 ` [PATCH v2 10/12] dt-bindings: power: Add MT8183 power domains Enric Balletbo i Serra
2020-10-01 16:01 ` [PATCH v2 11/12] soc: mediatek: pm-domains: Add support for mt8183 Enric Balletbo i Serra
2020-10-01 16:01 ` [PATCH v2 12/12] arm64: dts: mediatek: Add mt8183 power domains controller Enric Balletbo i Serra
2020-10-02  9:11   ` Matthias Brugger
2020-10-11 14:55   ` kernel test robot

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