2015-12-30 06:42:01

by James Liao

[permalink] [raw]
Subject: [PATCH 0/4] Mediatek MT2701 SCPSYS power domain support

This patchset is based on 4.4-rc7, add scpsys power domain support for
Mediatek MT2701/MT7623.

This patchset also separate MT8173 scpsys driver into common part
(mtk-scpsys.c) and platform part (mtk-scpsys-mt8173.c), so that MT2701
scpsys driver can share most implementation with MT8173.

James Liao (2):
soc: mediatek: Separate scpsys driver common code
soc: mediatek: Init MT8173 scpsys driver earlier

Shunli Wang (2):
soc: mediatek: Add MT2701 power dt-bindings
soc: mediatek: Add MT2701/MT7623 scpsys driver

drivers/soc/mediatek/Kconfig | 24 ++-
drivers/soc/mediatek/Makefile | 2 +
drivers/soc/mediatek/mtk-scpsys-mt2701.c | 161 +++++++++++++++++
drivers/soc/mediatek/mtk-scpsys-mt8173.c | 190 +++++++++++++++++++
drivers/soc/mediatek/mtk-scpsys.c | 301 ++++++++-----------------------
drivers/soc/mediatek/mtk-scpsys.h | 54 ++++++
include/dt-bindings/power/mt2701-power.h | 27 +++
7 files changed, 531 insertions(+), 228 deletions(-)
create mode 100644 drivers/soc/mediatek/mtk-scpsys-mt2701.c
create mode 100644 drivers/soc/mediatek/mtk-scpsys-mt8173.c
create mode 100644 drivers/soc/mediatek/mtk-scpsys.h
create mode 100644 include/dt-bindings/power/mt2701-power.h

--
1.9.1


2015-12-30 06:43:05

by James Liao

[permalink] [raw]
Subject: [PATCH 1/4] soc: mediatek: Separate scpsys driver common code

Separate scpsys driver common code to mtk-scpsys.c, and move MT8173
platform code to mtk-scpsys-mt8173.c.

Signed-off-by: James Liao <[email protected]>
---
drivers/soc/mediatek/Kconfig | 13 +-
drivers/soc/mediatek/Makefile | 1 +
drivers/soc/mediatek/mtk-scpsys-mt8173.c | 179 ++++++++++++++++++
drivers/soc/mediatek/mtk-scpsys.c | 301 ++++++++-----------------------
drivers/soc/mediatek/mtk-scpsys.h | 54 ++++++
5 files changed, 320 insertions(+), 228 deletions(-)
create mode 100644 drivers/soc/mediatek/mtk-scpsys-mt8173.c
create mode 100644 drivers/soc/mediatek/mtk-scpsys.h

diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
index 0a4ea80..eca6fb7 100644
--- a/drivers/soc/mediatek/Kconfig
+++ b/drivers/soc/mediatek/Kconfig
@@ -22,11 +22,20 @@ config MTK_PMIC_WRAP

config MTK_SCPSYS
bool "MediaTek SCPSYS Support"
- depends on ARCH_MEDIATEK || COMPILE_TEST
- default ARM64 && ARCH_MEDIATEK
select REGMAP
select MTK_INFRACFG
select PM_GENERIC_DOMAINS if PM
help
Say yes here to add support for the MediaTek SCPSYS power domain
driver.
+
+config MTK_SCPSYS_MT8173
+ bool "MediaTek MT8173 SCPSYS Support"
+ depends on ARCH_MEDIATEK || COMPILE_TEST
+ select MTK_SCPSYS
+ default ARCH_MEDIATEK
+ help
+ Say yes here to add support for the MT8173 SCPSYS power domain
+ driver.
+ The System Control Processor System (SCPSYS) has several power
+ management related tasks in the system.
diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
index 12998b0..3b22baa 100644
--- a/drivers/soc/mediatek/Makefile
+++ b/drivers/soc/mediatek/Makefile
@@ -1,3 +1,4 @@
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_MT8173) += mtk-scpsys-mt8173.o
diff --git a/drivers/soc/mediatek/mtk-scpsys-mt8173.c b/drivers/soc/mediatek/mtk-scpsys-mt8173.c
new file mode 100644
index 0000000..3c7b569
--- /dev/null
+++ b/drivers/soc/mediatek/mtk-scpsys-mt8173.c
@@ -0,0 +1,179 @@
+/*
+ * Copyright (c) 2015 Pengutronix, Sascha Hauer <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/pm_domain.h>
+#include <linux/soc/mediatek/infracfg.h>
+#include <dt-bindings/power/mt8173-power.h>
+
+#include "mtk-scpsys.h"
+
+#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 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)
+
+static const struct scp_domain_data scp_domain_data[] __initconst = {
+ [MT8173_POWER_DOMAIN_VDEC] = {
+ .name = "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),
+ .clk_id = {CLK_MM},
+ },
+ [MT8173_POWER_DOMAIN_VENC] = {
+ .name = "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),
+ .clk_id = {CLK_MM, CLK_VENC},
+ },
+ [MT8173_POWER_DOMAIN_ISP] = {
+ .name = "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),
+ .clk_id = {CLK_MM},
+ },
+ [MT8173_POWER_DOMAIN_MM] = {
+ .name = "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),
+ .clk_id = {CLK_MM},
+ .bus_prot_mask = MT8173_TOP_AXI_PROT_EN_MM_M0 |
+ MT8173_TOP_AXI_PROT_EN_MM_M1,
+ },
+ [MT8173_POWER_DOMAIN_VENC_LT] = {
+ .name = "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),
+ .clk_id = {CLK_MM, CLK_VENC_LT},
+ },
+ [MT8173_POWER_DOMAIN_AUDIO] = {
+ .name = "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),
+ .clk_id = {CLK_NONE},
+ },
+ [MT8173_POWER_DOMAIN_USB] = {
+ .name = "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),
+ .clk_id = {CLK_NONE},
+ .active_wakeup = true,
+ },
+ [MT8173_POWER_DOMAIN_MFG_ASYNC] = {
+ .name = "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,
+ .clk_id = {CLK_MFG},
+ },
+ [MT8173_POWER_DOMAIN_MFG_2D] = {
+ .name = "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),
+ .clk_id = {CLK_NONE},
+ },
+ [MT8173_POWER_DOMAIN_MFG] = {
+ .name = "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),
+ .clk_id = {CLK_NONE},
+ .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,
+ },
+};
+
+#define NUM_DOMAINS ARRAY_SIZE(scp_domain_data)
+
+static int __init scpsys_probe(struct platform_device *pdev)
+{
+ struct scp *scp;
+ struct genpd_onecell_data *pd_data;
+ int ret;
+
+ scp = init_scp(pdev, scp_domain_data, NUM_DOMAINS);
+ if (IS_ERR(scp))
+ return PTR_ERR(scp);
+
+ mtk_register_power_domains(pdev, scp, NUM_DOMAINS);
+
+ pd_data = &scp->pd_data;
+
+ ret = pm_genpd_add_subdomain(pd_data->domains[MT8173_POWER_DOMAIN_MFG_ASYNC],
+ pd_data->domains[MT8173_POWER_DOMAIN_MFG_2D]);
+ if (ret && IS_ENABLED(CONFIG_PM))
+ dev_err(&pdev->dev, "Failed to add subdomain: %d\n", ret);
+
+ ret = pm_genpd_add_subdomain(pd_data->domains[MT8173_POWER_DOMAIN_MFG_2D],
+ pd_data->domains[MT8173_POWER_DOMAIN_MFG]);
+ if (ret && IS_ENABLED(CONFIG_PM))
+ dev_err(&pdev->dev, "Failed to add subdomain: %d\n", ret);
+
+ return 0;
+}
+
+static const struct of_device_id of_scpsys_match_tbl[] = {
+ {
+ .compatible = "mediatek,mt8173-scpsys",
+ }, {
+ /* sentinel */
+ }
+};
+
+static struct platform_driver scpsys_drv = {
+ .driver = {
+ .name = "mtk-scpsys-mt8173",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(of_scpsys_match_tbl),
+ },
+};
+
+module_platform_driver_probe(scpsys_drv, scpsys_probe);
diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
index 4d4203c..a0943c5 100644
--- a/drivers/soc/mediatek/mtk-scpsys.c
+++ b/drivers/soc/mediatek/mtk-scpsys.c
@@ -11,28 +11,14 @@
* GNU General Public License for more details.
*/
#include <linux/clk.h>
-#include <linux/delay.h>
#include <linux/io.h>
-#include <linux/kernel.h>
#include <linux/mfd/syscon.h>
-#include <linux/module.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 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
+
+#include "mtk-scpsys.h"
+
#define SPM_PWR_STATUS 0x060c
#define SPM_PWR_STATUS_2ND 0x0610

@@ -42,153 +28,6 @@
#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)
-
-enum clk_id {
- MT8173_CLK_NONE,
- MT8173_CLK_MM,
- MT8173_CLK_MFG,
- MT8173_CLK_VENC,
- MT8173_CLK_VENC_LT,
- MT8173_CLK_MAX,
-};
-
-#define MAX_CLKS 2
-
-struct scp_domain_data {
- const char *name;
- u32 sta_mask;
- int ctl_offs;
- u32 sram_pdn_bits;
- u32 sram_pdn_ack_bits;
- u32 bus_prot_mask;
- enum clk_id clk_id[MAX_CLKS];
- bool active_wakeup;
-};
-
-static const struct scp_domain_data scp_domain_data[] __initconst = {
- [MT8173_POWER_DOMAIN_VDEC] = {
- .name = "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),
- .clk_id = {MT8173_CLK_MM},
- },
- [MT8173_POWER_DOMAIN_VENC] = {
- .name = "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),
- .clk_id = {MT8173_CLK_MM, MT8173_CLK_VENC},
- },
- [MT8173_POWER_DOMAIN_ISP] = {
- .name = "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),
- .clk_id = {MT8173_CLK_MM},
- },
- [MT8173_POWER_DOMAIN_MM] = {
- .name = "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),
- .clk_id = {MT8173_CLK_MM},
- .bus_prot_mask = MT8173_TOP_AXI_PROT_EN_MM_M0 |
- MT8173_TOP_AXI_PROT_EN_MM_M1,
- },
- [MT8173_POWER_DOMAIN_VENC_LT] = {
- .name = "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),
- .clk_id = {MT8173_CLK_MM, MT8173_CLK_VENC_LT},
- },
- [MT8173_POWER_DOMAIN_AUDIO] = {
- .name = "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),
- .clk_id = {MT8173_CLK_NONE},
- },
- [MT8173_POWER_DOMAIN_USB] = {
- .name = "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),
- .clk_id = {MT8173_CLK_NONE},
- .active_wakeup = true,
- },
- [MT8173_POWER_DOMAIN_MFG_ASYNC] = {
- .name = "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,
- .clk_id = {MT8173_CLK_MFG},
- },
- [MT8173_POWER_DOMAIN_MFG_2D] = {
- .name = "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),
- .clk_id = {MT8173_CLK_NONE},
- },
- [MT8173_POWER_DOMAIN_MFG] = {
- .name = "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),
- .clk_id = {MT8173_CLK_NONE},
- .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,
- },
-};
-
-#define NUM_DOMAINS ARRAY_SIZE(scp_domain_data)
-
-struct scp;
-
-struct scp_domain {
- struct generic_pm_domain genpd;
- struct scp *scp;
- struct clk *clk[MAX_CLKS];
- u32 sta_mask;
- void __iomem *ctl_addr;
- u32 sram_pdn_bits;
- u32 sram_pdn_ack_bits;
- u32 bus_prot_mask;
- bool active_wakeup;
-};
-
-struct scp {
- struct scp_domain domains[NUM_DOMAINS];
- struct genpd_onecell_data pd_data;
- struct device *dev;
- void __iomem *base;
- struct regmap *infracfg;
-};
-
static int scpsys_domain_is_on(struct scp_domain *scpd)
{
struct scp *scp = scpd->scp;
@@ -398,63 +237,89 @@ static bool scpsys_active_wakeup(struct device *dev)
return scpd->active_wakeup;
}

-static int __init scpsys_probe(struct platform_device *pdev)
+static void init_clks(struct platform_device *pdev, struct clk *clk[CLK_MAX])
+{
+ enum clk_id clk_ids[] = {
+ CLK_MM,
+ CLK_MFG,
+ CLK_VENC,
+ CLK_VENC_LT
+ };
+
+ static const char * const clk_names[] = {
+ "mm",
+ "mfg",
+ "venc",
+ "venc_lt",
+ };
+
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(clk_ids); i++)
+ clk[clk_ids[i]] = devm_clk_get(&pdev->dev, clk_names[i]);
+}
+
+struct scp *init_scp(struct platform_device *pdev,
+ const struct scp_domain_data *scp_domain_data, int num)
{
struct genpd_onecell_data *pd_data;
struct resource *res;
- int i, j, ret;
+ int i, j;
struct scp *scp;
- struct clk *clk[MT8173_CLK_MAX];
+ struct clk *clk[CLK_MAX];

scp = devm_kzalloc(&pdev->dev, sizeof(*scp), GFP_KERNEL);
if (!scp)
- return -ENOMEM;
+ return ERR_PTR(-ENOMEM);

scp->dev = &pdev->dev;

res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
scp->base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(scp->base))
- return PTR_ERR(scp->base);
-
- pd_data = &scp->pd_data;
-
- pd_data->domains = devm_kzalloc(&pdev->dev,
- sizeof(*pd_data->domains) * NUM_DOMAINS, GFP_KERNEL);
- if (!pd_data->domains)
- return -ENOMEM;
-
- clk[MT8173_CLK_MM] = devm_clk_get(&pdev->dev, "mm");
- if (IS_ERR(clk[MT8173_CLK_MM]))
- return PTR_ERR(clk[MT8173_CLK_MM]);
-
- clk[MT8173_CLK_MFG] = devm_clk_get(&pdev->dev, "mfg");
- if (IS_ERR(clk[MT8173_CLK_MFG]))
- return PTR_ERR(clk[MT8173_CLK_MFG]);
-
- clk[MT8173_CLK_VENC] = devm_clk_get(&pdev->dev, "venc");
- if (IS_ERR(clk[MT8173_CLK_VENC]))
- return PTR_ERR(clk[MT8173_CLK_VENC]);
-
- clk[MT8173_CLK_VENC_LT] = devm_clk_get(&pdev->dev, "venc_lt");
- if (IS_ERR(clk[MT8173_CLK_VENC_LT]))
- return PTR_ERR(clk[MT8173_CLK_VENC_LT]);
+ return ERR_CAST(scp->base);

scp->infracfg = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
"infracfg");
if (IS_ERR(scp->infracfg)) {
dev_err(&pdev->dev, "Cannot find infracfg controller: %ld\n",
PTR_ERR(scp->infracfg));
- return PTR_ERR(scp->infracfg);
+ return ERR_CAST(scp->infracfg);
}

- pd_data->num_domains = NUM_DOMAINS;
+ scp->domains = devm_kzalloc(&pdev->dev,
+ sizeof(*scp->domains) * num, GFP_KERNEL);
+ if (!scp->domains)
+ return ERR_PTR(-ENOMEM);
+
+ pd_data = &scp->pd_data;
+
+ pd_data->domains = devm_kzalloc(&pdev->dev,
+ sizeof(*pd_data->domains) * num, GFP_KERNEL);
+ if (!pd_data->domains)
+ return ERR_PTR(-ENOMEM);

- for (i = 0; i < NUM_DOMAINS; i++) {
+ pd_data->num_domains = num;
+
+ init_clks(pdev, clk);
+
+ for (i = 0; i < num; i++) {
struct scp_domain *scpd = &scp->domains[i];
struct generic_pm_domain *genpd = &scpd->genpd;
const struct scp_domain_data *data = &scp_domain_data[i];

+ for (j = 0; j < MAX_CLKS && data->clk_id[j]; j++) {
+ struct clk *c = clk[data->clk_id[j]];
+
+ if (IS_ERR(c)) {
+ dev_err(&pdev->dev, "%s: clk unavailable\n",
+ data->name);
+ return ERR_CAST(c);
+ }
+
+ scpd->clk[j] = c;
+ }
+
pd_data->domains[i] = genpd;
scpd->scp = scp;

@@ -464,13 +329,25 @@ static int __init scpsys_probe(struct platform_device *pdev)
scpd->sram_pdn_ack_bits = data->sram_pdn_ack_bits;
scpd->bus_prot_mask = data->bus_prot_mask;
scpd->active_wakeup = data->active_wakeup;
- for (j = 0; j < MAX_CLKS && data->clk_id[j]; j++)
- scpd->clk[j] = clk[data->clk_id[j]];

genpd->name = data->name;
genpd->power_off = scpsys_power_off;
genpd->power_on = scpsys_power_on;
genpd->dev_ops.active_wakeup = scpsys_active_wakeup;
+ }
+
+ return scp;
+}
+
+void mtk_register_power_domains(struct platform_device *pdev,
+ struct scp *scp, int num)
+{
+ struct genpd_onecell_data *pd_data;
+ int i, ret;
+
+ for (i = 0; i < num; i++) {
+ struct scp_domain *scpd = &scp->domains[i];
+ struct generic_pm_domain *genpd = &scpd->genpd;

/*
* Initially turn on all domains to make the domains usable
@@ -489,37 +366,9 @@ static int __init scpsys_probe(struct platform_device *pdev)
* valid.
*/

- ret = pm_genpd_add_subdomain(pd_data->domains[MT8173_POWER_DOMAIN_MFG_ASYNC],
- pd_data->domains[MT8173_POWER_DOMAIN_MFG_2D]);
- if (ret && IS_ENABLED(CONFIG_PM))
- dev_err(&pdev->dev, "Failed to add subdomain: %d\n", ret);
-
- ret = pm_genpd_add_subdomain(pd_data->domains[MT8173_POWER_DOMAIN_MFG_2D],
- pd_data->domains[MT8173_POWER_DOMAIN_MFG]);
- if (ret && IS_ENABLED(CONFIG_PM))
- dev_err(&pdev->dev, "Failed to add subdomain: %d\n", ret);
+ pd_data = &scp->pd_data;

ret = of_genpd_add_provider_onecell(pdev->dev.of_node, pd_data);
if (ret)
dev_err(&pdev->dev, "Failed to add OF provider: %d\n", ret);
-
- return 0;
}
-
-static const struct of_device_id of_scpsys_match_tbl[] = {
- {
- .compatible = "mediatek,mt8173-scpsys",
- }, {
- /* sentinel */
- }
-};
-
-static struct platform_driver scpsys_drv = {
- .driver = {
- .name = "mtk-scpsys",
- .owner = THIS_MODULE,
- .of_match_table = of_match_ptr(of_scpsys_match_tbl),
- },
-};
-
-module_platform_driver_probe(scpsys_drv, scpsys_probe);
diff --git a/drivers/soc/mediatek/mtk-scpsys.h b/drivers/soc/mediatek/mtk-scpsys.h
new file mode 100644
index 0000000..466728d
--- /dev/null
+++ b/drivers/soc/mediatek/mtk-scpsys.h
@@ -0,0 +1,54 @@
+#ifndef __DRV_SOC_MTK_H
+#define __DRV_SOC_MTK_H
+
+enum clk_id {
+ CLK_NONE,
+ CLK_MM,
+ CLK_MFG,
+ CLK_VENC,
+ CLK_VENC_LT,
+ CLK_MAX,
+};
+
+#define MAX_CLKS 2
+
+struct scp_domain_data {
+ const char *name;
+ u32 sta_mask;
+ int ctl_offs;
+ u32 sram_pdn_bits;
+ u32 sram_pdn_ack_bits;
+ u32 bus_prot_mask;
+ enum clk_id clk_id[MAX_CLKS];
+ bool active_wakeup;
+};
+
+struct scp;
+
+struct scp_domain {
+ struct generic_pm_domain genpd;
+ struct scp *scp;
+ struct clk *clk[MAX_CLKS];
+ u32 sta_mask;
+ void __iomem *ctl_addr;
+ u32 sram_pdn_bits;
+ u32 sram_pdn_ack_bits;
+ u32 bus_prot_mask;
+ bool active_wakeup;
+};
+
+struct scp {
+ struct scp_domain *domains;
+ struct genpd_onecell_data pd_data;
+ struct device *dev;
+ void __iomem *base;
+ struct regmap *infracfg;
+};
+
+struct scp *init_scp(struct platform_device *pdev,
+ const struct scp_domain_data *scp_domain_data, int num);
+
+void mtk_register_power_domains(struct platform_device *pdev,
+ struct scp *scp, int num);
+
+#endif /* __DRV_SOC_MTK_H */
--
1.9.1

2015-12-30 06:42:53

by James Liao

[permalink] [raw]
Subject: [PATCH 2/4] soc: mediatek: Init MT8173 scpsys driver earlier

Some power domain comsumers may init before module_init.
So the power domain provider (scpsys) need to be initialized
earlier too.

Signed-off-by: James Liao <[email protected]>
---
drivers/soc/mediatek/mtk-scpsys-mt8173.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/mediatek/mtk-scpsys-mt8173.c b/drivers/soc/mediatek/mtk-scpsys-mt8173.c
index 3c7b569..827e696 100644
--- a/drivers/soc/mediatek/mtk-scpsys-mt8173.c
+++ b/drivers/soc/mediatek/mtk-scpsys-mt8173.c
@@ -176,4 +176,15 @@ static struct platform_driver scpsys_drv = {
},
};

-module_platform_driver_probe(scpsys_drv, scpsys_probe);
+static int __init scpsys_drv_init(void)
+{
+ return platform_driver_probe(&scpsys_drv, scpsys_probe);
+}
+
+static void __exit scpsys_drv_exit(void)
+{
+ platform_driver_unregister(&scpsys_drv);
+}
+
+subsys_initcall(scpsys_drv_init);
+module_exit(scpsys_drv_exit);
--
1.9.1

2015-12-30 06:43:03

by James Liao

[permalink] [raw]
Subject: [PATCH 3/4] soc: mediatek: Add MT2701 power dt-bindings

From: Shunli Wang <[email protected]>

Add power dt-bindings for MT2701.

Signed-off-by: Shunli Wang <[email protected]>
Signed-off-by: James Liao <[email protected]>
---
include/dt-bindings/power/mt2701-power.h | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
create mode 100644 include/dt-bindings/power/mt2701-power.h

diff --git a/include/dt-bindings/power/mt2701-power.h b/include/dt-bindings/power/mt2701-power.h
new file mode 100644
index 0000000..64cc826
--- /dev/null
+++ b/include/dt-bindings/power/mt2701-power.h
@@ -0,0 +1,27 @@
+/*
+ * Copyright (C) 2015 MediaTek Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _DT_BINDINGS_POWER_MT2701_POWER_H
+#define _DT_BINDINGS_POWER_MT2701_POWER_H
+
+#define MT2701_POWER_DOMAIN_CONN 0
+#define MT2701_POWER_DOMAIN_DISP 1
+#define MT2701_POWER_DOMAIN_MFG 2
+#define MT2701_POWER_DOMAIN_VDEC 3
+#define MT2701_POWER_DOMAIN_ISP 4
+#define MT2701_POWER_DOMAIN_BDP 5
+#define MT2701_POWER_DOMAIN_ETH 6
+#define MT2701_POWER_DOMAIN_HIF 7
+#define MT2701_POWER_DOMAIN_IFR_MSC 8
+
+#endif /* _DT_BINDINGS_POWER_MT2701_POWER_H */
--
1.9.1

2015-12-30 06:42:56

by James Liao

[permalink] [raw]
Subject: [PATCH 4/4] soc: mediatek: Add MT2701/MT7623 scpsys driver

From: Shunli Wang <[email protected]>

Add scpsys driver for MT2701 and MT7623.

Signed-off-by: Shunli Wang <[email protected]>
Signed-off-by: James Liao <[email protected]>
---
drivers/soc/mediatek/Kconfig | 11 +++
drivers/soc/mediatek/Makefile | 1 +
drivers/soc/mediatek/mtk-scpsys-mt2701.c | 161 +++++++++++++++++++++++++++++++
3 files changed, 173 insertions(+)
create mode 100644 drivers/soc/mediatek/mtk-scpsys-mt2701.c

diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
index eca6fb7..92cf838 100644
--- a/drivers/soc/mediatek/Kconfig
+++ b/drivers/soc/mediatek/Kconfig
@@ -39,3 +39,14 @@ config MTK_SCPSYS_MT8173
driver.
The System Control Processor System (SCPSYS) has several power
management related tasks in the system.
+
+config MTK_SCPSYS_MT2701
+ bool "SCPSYS Support MediaTek MT2701 and MT7623"
+ depends on ARCH_MEDIATEK || COMPILE_TEST
+ select MTK_SCPSYS
+ default ARCH_MEDIATEK
+ help
+ Say yes here to add support for the MT2701/MT7623 SCPSYS power
+ domain driver.
+ The System Control Processor System (SCPSYS) has several power
+ management related tasks in the system.
diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
index 3b22baa..822986d 100644
--- a/drivers/soc/mediatek/Makefile
+++ b/drivers/soc/mediatek/Makefile
@@ -2,3 +2,4 @@ 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_MT8173) += mtk-scpsys-mt8173.o
+obj-$(CONFIG_MTK_SCPSYS_MT2701) += mtk-scpsys-mt2701.o
diff --git a/drivers/soc/mediatek/mtk-scpsys-mt2701.c b/drivers/soc/mediatek/mtk-scpsys-mt2701.c
new file mode 100644
index 0000000..339d5b8
--- /dev/null
+++ b/drivers/soc/mediatek/mtk-scpsys-mt2701.c
@@ -0,0 +1,161 @@
+/*
+ * Copyright (c) 2015 Mediatek, Shunli Wang <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/pm_domain.h>
+#include <linux/soc/mediatek/infracfg.h>
+#include <dt-bindings/power/mt2701-power.h>
+
+#include "mtk-scpsys.h"
+
+#define SPM_VDE_PWR_CON 0x0210
+#define SPM_MFG_PWR_CON 0x0214
+#define SPM_ISP_PWR_CON 0x0238
+#define SPM_DIS_PWR_CON 0x023C
+#define SPM_CONN_PWR_CON 0x0280
+#define SPM_BDP_PWR_CON 0x029C
+#define SPM_ETH_PWR_CON 0x02A0
+#define SPM_HIF_PWR_CON 0x02A4
+#define SPM_IFR_MSC_PWR_CON 0x02A8
+#define SPM_PWR_STATUS 0x060c
+#define SPM_PWR_STATUS_2ND 0x0610
+
+#define CONN_PWR_STA_MASK BIT(1)
+#define DIS_PWR_STA_MASK BIT(3)
+#define MFG_PWR_STA_MASK BIT(4)
+#define ISP_PWR_STA_MASK BIT(5)
+#define VDE_PWR_STA_MASK BIT(7)
+#define BDP_PWR_STA_MASK BIT(14)
+#define ETH_PWR_STA_MASK BIT(15)
+#define HIF_PWR_STA_MASK BIT(16)
+#define IFR_MSC_PWR_STA_MASK BIT(17)
+
+#define MT2701_TOP_AXI_PROT_EN_CONN 0x0104
+#define MT2701_TOP_AXI_PROT_EN_DISP 0x0002
+
+static const struct scp_domain_data scp_domain_data[] = {
+ [MT2701_POWER_DOMAIN_CONN] = {
+ .name = "conn",
+ .sta_mask = CONN_PWR_STA_MASK,
+ .ctl_offs = SPM_CONN_PWR_CON,
+ .bus_prot_mask = MT2701_TOP_AXI_PROT_EN_CONN,
+ .active_wakeup = true,
+ },
+ [MT2701_POWER_DOMAIN_DISP] = {
+ .name = "disp",
+ .sta_mask = DIS_PWR_STA_MASK,
+ .ctl_offs = SPM_DIS_PWR_CON,
+ .sram_pdn_bits = GENMASK(11, 8),
+ .clk_id = {CLK_MM},
+ .bus_prot_mask = MT2701_TOP_AXI_PROT_EN_DISP,
+ .active_wakeup = true,
+ },
+ [MT2701_POWER_DOMAIN_MFG] = {
+ .name = "mfg",
+ .sta_mask = MFG_PWR_STA_MASK,
+ .ctl_offs = SPM_MFG_PWR_CON,
+ .sram_pdn_bits = GENMASK(11, 8),
+ .sram_pdn_ack_bits = GENMASK(12, 12),
+ .active_wakeup = true,
+ },
+ [MT2701_POWER_DOMAIN_VDEC] = {
+ .name = "vdec",
+ .sta_mask = VDE_PWR_STA_MASK,
+ .ctl_offs = SPM_VDE_PWR_CON,
+ .sram_pdn_bits = GENMASK(11, 8),
+ .sram_pdn_ack_bits = GENMASK(12, 12),
+ .clk_id = {CLK_MM},
+ .active_wakeup = true,
+ },
+ [MT2701_POWER_DOMAIN_ISP] = {
+ .name = "isp",
+ .sta_mask = ISP_PWR_STA_MASK,
+ .ctl_offs = SPM_ISP_PWR_CON,
+ .sram_pdn_bits = GENMASK(11, 8),
+ .sram_pdn_ack_bits = GENMASK(13, 12),
+ .active_wakeup = true,
+ },
+ [MT2701_POWER_DOMAIN_BDP] = {
+ .name = "bdp",
+ .sta_mask = BDP_PWR_STA_MASK,
+ .ctl_offs = SPM_BDP_PWR_CON,
+ .sram_pdn_bits = GENMASK(11, 8),
+ .active_wakeup = true,
+ },
+ [MT2701_POWER_DOMAIN_ETH] = {
+ .name = "eth",
+ .sta_mask = ETH_PWR_STA_MASK,
+ .ctl_offs = SPM_ETH_PWR_CON,
+ .sram_pdn_bits = GENMASK(11, 8),
+ .sram_pdn_ack_bits = GENMASK(15, 12),
+ .active_wakeup = true,
+ },
+ [MT2701_POWER_DOMAIN_HIF] = {
+ .name = "hif",
+ .sta_mask = HIF_PWR_STA_MASK,
+ .ctl_offs = SPM_HIF_PWR_CON,
+ .sram_pdn_bits = GENMASK(11, 8),
+ .sram_pdn_ack_bits = GENMASK(15, 12),
+ .active_wakeup = true,
+ },
+ [MT2701_POWER_DOMAIN_IFR_MSC] = {
+ .name = "ifr_msc",
+ .sta_mask = IFR_MSC_PWR_STA_MASK,
+ .ctl_offs = SPM_IFR_MSC_PWR_CON,
+ .active_wakeup = true,
+ },
+};
+
+#define NUM_DOMAINS ARRAY_SIZE(scp_domain_data)
+
+static int __init scpsys_probe(struct platform_device *pdev)
+{
+ struct scp *scp;
+
+ scp = init_scp(pdev, scp_domain_data, NUM_DOMAINS);
+ if (IS_ERR(scp))
+ return PTR_ERR(scp);
+
+ mtk_register_power_domains(pdev, scp, NUM_DOMAINS);
+
+ return 0;
+}
+
+static const struct of_device_id of_scpsys_match_tbl[] = {
+ {
+ .compatible = "mediatek,mt2701-scpsys",
+ }, {
+ /* sentinel */
+ }
+};
+MODULE_DEVICE_TABLE(of, of_scpsys_match_tbl);
+
+static struct platform_driver scpsys_drv = {
+ .driver = {
+ .name = "mtk-scpsys-mt2701",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(of_scpsys_match_tbl),
+ },
+ .probe = scpsys_probe,
+};
+
+static int __init scpsys_init(void)
+{
+ return platform_driver_register(&scpsys_drv);
+}
+
+subsys_initcall(scpsys_init);
+
+MODULE_DESCRIPTION("MediaTek MT2701 scpsys driver");
+MODULE_LICENSE("GPL v2");
--
1.9.1

2015-12-30 08:53:17

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 1/4] soc: mediatek: Separate scpsys driver common code

On Wednesday 30 December 2015 14:41:43 James Liao wrote:
> diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
> index 0a4ea80..eca6fb7 100644
> --- a/drivers/soc/mediatek/Kconfig
> +++ b/drivers/soc/mediatek/Kconfig
> @@ -22,11 +22,20 @@ config MTK_PMIC_WRAP
>
> config MTK_SCPSYS
> bool "MediaTek SCPSYS Support"
> - depends on ARCH_MEDIATEK || COMPILE_TEST
> - default ARM64 && ARCH_MEDIATEK
> select REGMAP
> select MTK_INFRACFG
> select PM_GENERIC_DOMAINS if PM
> help
> Say yes here to add support for the MediaTek SCPSYS power domain
> driver.
> +
> +config MTK_SCPSYS_MT8173
> + bool "MediaTek MT8173 SCPSYS Support"
> + depends on ARCH_MEDIATEK || COMPILE_TEST
> + select MTK_SCPSYS
> + default ARCH_MEDIATEK
>

Please don't "select" a user-visible Kconfig symbol. Either hide MTK_SCPSYS
by removing the string behind 'bool', or make this "depends on MTK_SCPSYS".

Arnd

2015-12-30 08:53:51

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 2/4] soc: mediatek: Init MT8173 scpsys driver earlier

On Wednesday 30 December 2015 14:41:44 James Liao wrote:
> Some power domain comsumers may init before module_init.
> So the power domain provider (scpsys) need to be initialized
> earlier too.
>
> Signed-off-by: James Liao <[email protected]>
> ---
>

Why?

Arnd

2015-12-30 09:02:21

by Daniel Kurtz

[permalink] [raw]
Subject: Re: [PATCH 4/4] soc: mediatek: Add MT2701/MT7623 scpsys driver

On Wed, Dec 30, 2015 at 2:41 PM, James Liao <[email protected]> wrote:
>
> From: Shunli Wang <[email protected]>
>
> Add scpsys driver for MT2701 and MT7623.
>
> Signed-off-by: Shunli Wang <[email protected]>
> Signed-off-by: James Liao <[email protected]>
> ---
> drivers/soc/mediatek/Kconfig | 11 +++
> drivers/soc/mediatek/Makefile | 1 +
> drivers/soc/mediatek/mtk-scpsys-mt2701.c | 161 +++++++++++++++++++++++++++++++
> 3 files changed, 173 insertions(+)
> create mode 100644 drivers/soc/mediatek/mtk-scpsys-mt2701.c
>
> diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
> index eca6fb7..92cf838 100644
> --- a/drivers/soc/mediatek/Kconfig
> +++ b/drivers/soc/mediatek/Kconfig
> @@ -39,3 +39,14 @@ config MTK_SCPSYS_MT8173
> driver.
> The System Control Processor System (SCPSYS) has several power
> management related tasks in the system.
> +
> +config MTK_SCPSYS_MT2701
> + bool "SCPSYS Support MediaTek MT2701 and MT7623"
> + depends on ARCH_MEDIATEK || COMPILE_TEST
> + select MTK_SCPSYS
> + default ARCH_MEDIATEK
> + help
> + Say yes here to add support for the MT2701/MT7623 SCPSYS power
> + domain driver.
> + The System Control Processor System (SCPSYS) has several power
> + management related tasks in the system.

I don't think we really want different drivers and Kconfig options.

Can we just use different compatibles to select the appropriate scp_domain_data?

-Dan

2015-12-30 10:09:11

by James Liao

[permalink] [raw]
Subject: Re: [PATCH 1/4] soc: mediatek: Separate scpsys driver common code

Hi Arnd,

On Wed, 2015-12-30 at 09:52 +0100, Arnd Bergmann wrote:
> On Wednesday 30 December 2015 14:41:43 James Liao wrote:
> > diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
> > index 0a4ea80..eca6fb7 100644
> > --- a/drivers/soc/mediatek/Kconfig
> > +++ b/drivers/soc/mediatek/Kconfig
> > @@ -22,11 +22,20 @@ config MTK_PMIC_WRAP
> >
> > config MTK_SCPSYS
> > bool "MediaTek SCPSYS Support"
> > - depends on ARCH_MEDIATEK || COMPILE_TEST
> > - default ARM64 && ARCH_MEDIATEK
> > select REGMAP
> > select MTK_INFRACFG
> > select PM_GENERIC_DOMAINS if PM
> > help
> > Say yes here to add support for the MediaTek SCPSYS power domain
> > driver.
> > +
> > +config MTK_SCPSYS_MT8173
> > + bool "MediaTek MT8173 SCPSYS Support"
> > + depends on ARCH_MEDIATEK || COMPILE_TEST
> > + select MTK_SCPSYS
> > + default ARCH_MEDIATEK
> >
>
> Please don't "select" a user-visible Kconfig symbol. Either hide MTK_SCPSYS
> by removing the string behind 'bool', or make this "depends on MTK_SCPSYS".

It looks something wrong during cherry-pick. MTK_SCPSYS should be
invisible from user. I'll remove the string behind 'bool' in next patch.


Best regards,

James

2015-12-30 10:12:40

by James Liao

[permalink] [raw]
Subject: Re: [PATCH 2/4] soc: mediatek: Init MT8173 scpsys driver earlier

Hi Arnd,

On Wed, 2015-12-30 at 09:52 +0100, Arnd Bergmann wrote:
> On Wednesday 30 December 2015 14:41:44 James Liao wrote:
> > Some power domain comsumers may init before module_init.
> > So the power domain provider (scpsys) need to be initialized
> > earlier too.
> >
> > Signed-off-by: James Liao <[email protected]>
> > ---
> >
>
> Why?

Some drivers use different init level to ensure they can be initialized
before other drivers. To support these drivers, moving scpsys driver's
initial function to subsys_init is the most easy way.



Best regards,

James

2015-12-30 10:18:20

by James Liao

[permalink] [raw]
Subject: Re: [PATCH 4/4] soc: mediatek: Add MT2701/MT7623 scpsys driver

Hi Daniel,

On Wed, 2015-12-30 at 17:01 +0800, Daniel Kurtz wrote:
> On Wed, Dec 30, 2015 at 2:41 PM, James Liao <[email protected]> wrote:
> >
> > From: Shunli Wang <[email protected]>
> >
> > Add scpsys driver for MT2701 and MT7623.
> >
> > Signed-off-by: Shunli Wang <[email protected]>
> > Signed-off-by: James Liao <[email protected]>
> > ---
> > drivers/soc/mediatek/Kconfig | 11 +++
> > drivers/soc/mediatek/Makefile | 1 +
> > drivers/soc/mediatek/mtk-scpsys-mt2701.c | 161 +++++++++++++++++++++++++++++++
> > 3 files changed, 173 insertions(+)
> > create mode 100644 drivers/soc/mediatek/mtk-scpsys-mt2701.c
> >
> > diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
> > index eca6fb7..92cf838 100644
> > --- a/drivers/soc/mediatek/Kconfig
> > +++ b/drivers/soc/mediatek/Kconfig
> > @@ -39,3 +39,14 @@ config MTK_SCPSYS_MT8173
> > driver.
> > The System Control Processor System (SCPSYS) has several power
> > management related tasks in the system.
> > +
> > +config MTK_SCPSYS_MT2701
> > + bool "SCPSYS Support MediaTek MT2701 and MT7623"
> > + depends on ARCH_MEDIATEK || COMPILE_TEST
> > + select MTK_SCPSYS
> > + default ARCH_MEDIATEK
> > + help
> > + Say yes here to add support for the MT2701/MT7623 SCPSYS power
> > + domain driver.
> > + The System Control Processor System (SCPSYS) has several power
> > + management related tasks in the system.
>
> I don't think we really want different drivers and Kconfig options.
>
> Can we just use different compatibles to select the appropriate scp_domain_data?

Yes, we can. All scpsys drivers are built-in by default, and they will
be activate by specific compatible string.

But some projects don't want to build unused drivers into kernel to save
code size. Use different Kconfig options for each SoC so that these
projects can disable unused drivers.


Best regards,

James

2015-12-30 10:36:04

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 2/4] soc: mediatek: Init MT8173 scpsys driver earlier

On Wednesday 30 December 2015 18:12:08 James Liao wrote:
> On Wed, 2015-12-30 at 09:52 +0100, Arnd Bergmann wrote:
> > On Wednesday 30 December 2015 14:41:44 James Liao wrote:
> > > Some power domain comsumers may init before module_init.
> > > So the power domain provider (scpsys) need to be initialized
> > > earlier too.
> > >
> > > Signed-off-by: James Liao <[email protected]>
> > > ---
> > >
> >
> > Why?
>
> Some drivers use different init level to ensure they can be initialized
> before other drivers. To support these drivers, moving scpsys driver's
> initial function to subsys_init is the most easy way.

This is just the same generic explanation that you already have.

Please be more specific what the dependency is and why we can't rely
on deferred probing here.

Arnd

2015-12-30 17:49:38

by Matthias Brugger

[permalink] [raw]
Subject: Re: [PATCH 4/4] soc: mediatek: Add MT2701/MT7623 scpsys driver



On 30/12/15 11:18, James Liao wrote:
> Hi Daniel,
>
> On Wed, 2015-12-30 at 17:01 +0800, Daniel Kurtz wrote:
>> On Wed, Dec 30, 2015 at 2:41 PM, James Liao <[email protected]> wrote:
>>>
>>> From: Shunli Wang <[email protected]>
>>>
>>> Add scpsys driver for MT2701 and MT7623.
>>>
>>> Signed-off-by: Shunli Wang <[email protected]>
>>> Signed-off-by: James Liao <[email protected]>
>>> ---
>>> drivers/soc/mediatek/Kconfig | 11 +++
>>> drivers/soc/mediatek/Makefile | 1 +
>>> drivers/soc/mediatek/mtk-scpsys-mt2701.c | 161 +++++++++++++++++++++++++++++++
>>> 3 files changed, 173 insertions(+)
>>> create mode 100644 drivers/soc/mediatek/mtk-scpsys-mt2701.c
>>>
>>> diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
>>> index eca6fb7..92cf838 100644
>>> --- a/drivers/soc/mediatek/Kconfig
>>> +++ b/drivers/soc/mediatek/Kconfig
>>> @@ -39,3 +39,14 @@ config MTK_SCPSYS_MT8173
>>> driver.
>>> The System Control Processor System (SCPSYS) has several power
>>> management related tasks in the system.
>>> +
>>> +config MTK_SCPSYS_MT2701
>>> + bool "SCPSYS Support MediaTek MT2701 and MT7623"
>>> + depends on ARCH_MEDIATEK || COMPILE_TEST
>>> + select MTK_SCPSYS
>>> + default ARCH_MEDIATEK
>>> + help
>>> + Say yes here to add support for the MT2701/MT7623 SCPSYS power
>>> + domain driver.
>>> + The System Control Processor System (SCPSYS) has several power
>>> + management related tasks in the system.
>>
>> I don't think we really want different drivers and Kconfig options.
>>
>> Can we just use different compatibles to select the appropriate scp_domain_data?
>
> Yes, we can. All scpsys drivers are built-in by default, and they will
> be activate by specific compatible string.
>
> But some projects don't want to build unused drivers into kernel to save
> code size. Use different Kconfig options for each SoC so that these
> projects can disable unused drivers.
>

Scpsys is a bool right now, you can disable it if you don't need it for
your project.

I don't think the impact of adding scp_domain_data justifies adding SoC
specific scpsys drivers and bloat the drivers/soc/mediatek folder.

Best regards,
Matthias

2015-12-31 05:59:33

by James Liao

[permalink] [raw]
Subject: Re: [PATCH 2/4] soc: mediatek: Init MT8173 scpsys driver earlier

Hi Arnd,

On Wed, 2015-12-30 at 11:35 +0100, Arnd Bergmann wrote:
> On Wednesday 30 December 2015 18:12:08 James Liao wrote:
> > On Wed, 2015-12-30 at 09:52 +0100, Arnd Bergmann wrote:
> > > On Wednesday 30 December 2015 14:41:44 James Liao wrote:
> > > > Some power domain comsumers may init before module_init.
> > > > So the power domain provider (scpsys) need to be initialized
> > > > earlier too.
> > > >
> > > > Signed-off-by: James Liao <[email protected]>
> > > > ---
> > > >
> > >
> > > Why?
> >
> > Some drivers use different init level to ensure they can be initialized
> > before other drivers. To support these drivers, moving scpsys driver's
> > initial function to subsys_init is the most easy way.
>
> This is just the same generic explanation that you already have.
>
> Please be more specific what the dependency is and why we can't rely
> on deferred probing here.

In our case, there is a SMI driver provide APIs to control multiple
devices that attached to different power domains.Video encoder / decoder
and GPU drivers are SMI users. It's not easy for SMI users to detect SMI
and scpsys driver are initialized or not. A most easy way to resolve the
init sequence issue is moving SMI and scpsys driver in early init stage.

Do you prefer to keep scpsys driver's init in module_init? If yes, I can
remove this patch in next version.


Best regards,

James

2015-12-31 09:16:47

by James Liao

[permalink] [raw]
Subject: Re: [PATCH 2/4] soc: mediatek: Init MT8173 scpsys driver earlier

Hi Arnd,
> On Wed, 2015-12-30 at 11:35 +0100, Arnd Bergmann wrote:
> > On Wednesday 30 December 2015 18:12:08 James Liao wrote:
> > > On Wed, 2015-12-30 at 09:52 +0100, Arnd Bergmann wrote:
> > > > On Wednesday 30 December 2015 14:41:44 James Liao wrote:
> > > > > Some power domain comsumers may init before module_init.
> > > > > So the power domain provider (scpsys) need to be initialized
> > > > > earlier too.
> > > > >
> > > > > Signed-off-by: James Liao <[email protected]>
> > > > > ---
> > > > >
> > > >
> > > > Why?
> > >
> > > Some drivers use different init level to ensure they can be initialized
> > > before other drivers. To support these drivers, moving scpsys driver's
> > > initial function to subsys_init is the most easy way.
> >
> > This is just the same generic explanation that you already have.
> >
> > Please be more specific what the dependency is and why we can't rely
> > on deferred probing here.
>
> In our case, there is a SMI driver provide APIs to control multiple
> devices that attached to different power domains.Video encoder / decoder
> and GPU drivers are SMI users. It's not easy for SMI users to detect SMI
> and scpsys driver are initialized or not. A most easy way to resolve the
> init sequence issue is moving SMI and scpsys driver in early init stage.
>
> Do you prefer to keep scpsys driver's init in module_init? If yes, I can
> remove this patch in next version.

After discuss with our SMI / IOMMU driver owner, he still prefer to keep
scpsys driver init in subsys_init. Here is his explanation:

"""
Take a example for our IOMMU(M4U) and SMI. The IOMMU which is
subsys_init defaultly will depend on SMI.
The SMI is a bridge between m4u and the Multimedia HW. About the HW
block diagram and more other information please help check [1].
SMI is responsible to enable/disable iommu and help transfer data for
each Multimedia HW, Both have to wait until the power and the clocks is
enabled.

So our iommu should probe done after smi, smi should be after
power-domain, and all the iommu consumer(display/vdec/venc/camera etc.)
should be after the iommu.
Then all the multimedia module will be delayed by power-domain who is
module_init currently.

After grep, we get some example whose pm is not module_init:
core_initcall(exynos4_pm_init_power_domain);
subsys_initcall(imx_pgc_init);

So we expect move the power-domain initial more earlier too. The
power-domain seems to be a basic module like ccf.
Is there some special reason about we should use module_init, or do you
have any concern if we change it?
Thanks.

[1]:
http://lists.linuxfoundation.org/pipermail/iommu/2015-December/015105.html


Best Regards.

James

2015-12-31 14:46:16

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 2/4] soc: mediatek: Init MT8173 scpsys driver earlier

On Thursday 31 December 2015 17:16:34 James Liao wrote:
>
> """
> Take a example for our IOMMU(M4U) and SMI. The IOMMU which is
> subsys_init defaultly will depend on SMI.
> The SMI is a bridge between m4u and the Multimedia HW. About the HW
> block diagram and more other information please help check [1].
> SMI is responsible to enable/disable iommu and help transfer data for
> each Multimedia HW, Both have to wait until the power and the clocks is
> enabled.
>
> So our iommu should probe done after smi, smi should be after
> power-domain, and all the iommu consumer(display/vdec/venc/camera etc.)
> should be after the iommu.
> Then all the multimedia module will be delayed by power-domain who is
> module_init currently.
>
> After grep, we get some example whose pm is not module_init:
> core_initcall(exynos4_pm_init_power_domain);
> subsys_initcall(imx_pgc_init);
>
> So we expect move the power-domain initial more earlier too. The
> power-domain seems to be a basic module like ccf.
> Is there some special reason about we should use module_init, or do you
> have any concern if we change it?
> Thanks.

Ok, got it. Generally, we should try to avoid using the earlier initcall
levels, but a few things like clock controllers, iommus etc are special
enough that we need to make sure their dependencies are there by the
time those are probed.

Please put your explanation above into the patch changelog and add a code
comment about the IOMMU next to the subsys_initcall() so it doesn't
accidentally get changed when someone tries to do a code cleanup.

Arnd