[PATCH V4 0/8] Pwrap: Mediatek pwrap driver for mt6797

[PATCH V4 0/8] Pwrap: Mediatek pwrap driver for mt6797

[argus.lin]

[-- Attachment #1.1: Type: text/html, Size: 2493 bytes --]

[-- Attachment #1.2: Type: text/plain, Size: 1555 bytes --]

From: Argus Lin <argus.lin@mediatek.com>

Here's version 4 of the patch series, include 8 patches:
1. add caps flag for pwrap to explain pwrap's capability.
2. remove has_bridge flag, replace by caps flag.
3. add int1_en_all flag for new interrupt.
4. add pwrap drivers for mt6797 SoCs.
5. fix bug at cipher init for
5ae48040aa47 ("soc: mediatek: PMIC wrap: add mt6323 slave support")
6. add mt6351 support for mt6797 SoCs.

Changes from v2 to v3:
1)adjust upstream patch sequence.
2)fix build error for new filed at pmic_wrapper_type.
3)adjust pmic_type enum sequence.

Changes from v1 to v2:
1) add new capability field and MARCO for different pwrap series, the capability include BRIDGE, RESET, DCM, PRIORITY_SEL and INT1_EN.
2) add starvaction for mt6797 pwrap.
3) add INT1 interrupt to handle starvaction and request exception.


Argus Lin (8):
  dt-bindings: pwrap: mediatek: add MT6351 PMIC support for MT6797
  soc: mediatek: pwrap: add caps flag for pwrap
  soc: mediatek: pwrap: remove has_bridge flag
  soc: mediatek: pwrap: add int1_en_all flag
  soc: mediatek: pwrap: add pwrap for mt6797 SoCs
  soc: mediatek: pwrap: fix cipher init
  soc: mediatek: pwrap: add mt6351 for mt6797 SoCs
  arm64: dts: mt6797: add pwrap support for mt6797

 .../devicetree/bindings/soc/mediatek/pwrap.txt     |   1 +
 arch/arm64/boot/dts/mediatek/mt6797.dtsi           |  14 ++
 drivers/soc/mediatek/mtk-pmic-wrap.c               | 167 ++++++++++++++++++---
 3 files changed, 165 insertions(+), 17 deletions(-)

-- 
2.12.5

[PATCH V4 1/8] dt-bindings: pwrap: mediatek: add MT6351 PMIC support for MT6797

[argus.lin]

[-- Attachment #1.1: Type: text/html, Size: 1845 bytes --]

[-- Attachment #1.2: Type: text/plain, Size: 869 bytes --]

From: Argus Lin <argus.lin@mediatek.com>

We add MT6351 PMIC support for MT6797 SoCs.

Signed-off-by: Argus Lin <argus.lin@mediatek.com>
---
 Documentation/devicetree/bindings/soc/mediatek/pwrap.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/soc/mediatek/pwrap.txt b/Documentation/devicetree/bindings/soc/mediatek/pwrap.txt
index bf80e3f96f8c..f9987c30f0d5 100644
--- a/Documentation/devicetree/bindings/soc/mediatek/pwrap.txt
+++ b/Documentation/devicetree/bindings/soc/mediatek/pwrap.txt
@@ -19,6 +19,7 @@ IP Pairing
 Required properties in pwrap device node.
 - compatible:
 	"mediatek,mt2701-pwrap" for MT2701/7623 SoCs
+	"mediatek,mt6797-pwrap" for MT6797 SoCs
 	"mediatek,mt7622-pwrap" for MT7622 SoCs
 	"mediatek,mt8135-pwrap" for MT8135 SoCs
 	"mediatek,mt8173-pwrap" for MT8173 SoCs
-- 
2.12.5

[PATCH V4 2/8] soc: mediatek: pwrap: add caps flag for pwrap

[argus.lin]

[-- Attachment #1.1: Type: text/html, Size: 3153 bytes --]

[-- Attachment #1.2: Type: text/plain, Size: 2219 bytes --]

From: Argus Lin <argus.lin@mediatek.com>

We use caps to describe pwrap's capability, used
to replace has_bridge flag for single meaning.

Signed-off-by: Argus Lin <argus.lin@mediatek.com>
---
 drivers/soc/mediatek/mtk-pmic-wrap.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index e9e054a15b7d..3a25ff6e6907 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -76,6 +76,12 @@
 #define PWRAP_SLV_CAP_SECURITY	BIT(2)
 #define HAS_CAP(_c, _x)	(((_c) & (_x)) == (_x))
 
+/* Group of bits used for shown pwrap capability */
+#define PWRAP_CAP_BRIDGE	BIT(0)
+#define PWRAP_CAP_RESET		BIT(1)
+#define PWRAP_CAP_DCM		BIT(2)
+#define PWRAP_CAP_INT1_EN	BIT(3)
+
 /* defines for slave device wrapper registers */
 enum dew_regs {
 	PWRAP_DEW_BASE,
@@ -684,6 +690,8 @@ struct pmic_wrapper_type {
 	u32 spi_w;
 	u32 wdt_src;
 	unsigned int has_bridge:1;
+	/* Flags indicating the capability for the target pwrap */
+	u8 caps;
 	int (*init_reg_clock)(struct pmic_wrapper *wrp);
 	int (*init_soc_specific)(struct pmic_wrapper *wrp);
 };
@@ -1394,6 +1402,7 @@ static const struct pmic_wrapper_type pwrap_mt2701 = {
 	.spi_w = PWRAP_MAN_CMD_SPI_WRITE_NEW,
 	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
 	.has_bridge = 0,
+	.caps = PWRAP_CAP_RESET | PWRAP_CAP_DCM,
 	.init_reg_clock = pwrap_mt2701_init_reg_clock,
 	.init_soc_specific = pwrap_mt2701_init_soc_specific,
 };
@@ -1406,6 +1415,7 @@ static const struct pmic_wrapper_type pwrap_mt7622 = {
 	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
 	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
 	.has_bridge = 0,
+	.caps = PWRAP_CAP_RESET | PWRAP_CAP_DCM,
 	.init_reg_clock = pwrap_common_init_reg_clock,
 	.init_soc_specific = pwrap_mt7622_init_soc_specific,
 };
@@ -1418,6 +1428,7 @@ static const struct pmic_wrapper_type pwrap_mt8135 = {
 	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
 	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
 	.has_bridge = 1,
+	.caps = PWRAP_CAP_BRIDGE | PWRAP_CAP_RESET | PWRAP_CAP_DCM,
 	.init_reg_clock = pwrap_common_init_reg_clock,
 	.init_soc_specific = pwrap_mt8135_init_soc_specific,
 };
-- 
2.12.5

[PATCH V4 3/8] soc: mediatek: pwrap: remove has_bridge flag

[argus.lin]

[-- Attachment #1.1: Type: text/html, Size: 3997 bytes --]

[-- Attachment #1.2: Type: text/plain, Size: 3025 bytes --]

From: Argus Lin <argus.lin@mediatek.com>

We use new flag caps to replace has_bridge.
Legacy chips support bridge use PWRAP_CAP_BRIDGE
to explain such capability.

Signed-off-by: Argus Lin <argus.lin@mediatek.com>
---
 drivers/soc/mediatek/mtk-pmic-wrap.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index 3a25ff6e6907..911a8508f39f 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -689,7 +689,6 @@ struct pmic_wrapper_type {
 	u32 int_en_all;
 	u32 spi_w;
 	u32 wdt_src;
-	unsigned int has_bridge:1;
 	/* Flags indicating the capability for the target pwrap */
 	u8 caps;
 	int (*init_reg_clock)(struct pmic_wrapper *wrp);
@@ -1306,7 +1305,7 @@ static int pwrap_init(struct pmic_wrapper *wrp)
 	pwrap_writel(wrp, 1, PWRAP_INIT_DONE0);
 	pwrap_writel(wrp, 1, PWRAP_INIT_DONE1);
 
-	if (wrp->master->has_bridge) {
+	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_BRIDGE)) {
 		writel(1, wrp->bridge_base + PWRAP_MT8135_BRIDGE_INIT_DONE3);
 		writel(1, wrp->bridge_base + PWRAP_MT8135_BRIDGE_INIT_DONE4);
 	}
@@ -1401,7 +1400,6 @@ static const struct pmic_wrapper_type pwrap_mt2701 = {
 	.int_en_all = ~(u32)(BIT(31) | BIT(2)),
 	.spi_w = PWRAP_MAN_CMD_SPI_WRITE_NEW,
 	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
-	.has_bridge = 0,
 	.caps = PWRAP_CAP_RESET | PWRAP_CAP_DCM,
 	.init_reg_clock = pwrap_mt2701_init_reg_clock,
 	.init_soc_specific = pwrap_mt2701_init_soc_specific,
@@ -1414,7 +1412,6 @@ static const struct pmic_wrapper_type pwrap_mt7622 = {
 	.int_en_all = ~(u32)BIT(31),
 	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
 	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
-	.has_bridge = 0,
 	.caps = PWRAP_CAP_RESET | PWRAP_CAP_DCM,
 	.init_reg_clock = pwrap_common_init_reg_clock,
 	.init_soc_specific = pwrap_mt7622_init_soc_specific,
@@ -1427,7 +1424,6 @@ static const struct pmic_wrapper_type pwrap_mt8135 = {
 	.int_en_all = ~(u32)(BIT(31) | BIT(1)),
 	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
 	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
-	.has_bridge = 1,
 	.caps = PWRAP_CAP_BRIDGE | PWRAP_CAP_RESET | PWRAP_CAP_DCM,
 	.init_reg_clock = pwrap_common_init_reg_clock,
 	.init_soc_specific = pwrap_mt8135_init_soc_specific,
@@ -1440,7 +1436,7 @@ static const struct pmic_wrapper_type pwrap_mt8173 = {
 	.int_en_all = ~(u32)(BIT(31) | BIT(1)),
 	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
 	.wdt_src = PWRAP_WDT_SRC_MASK_NO_STAUPD,
-	.has_bridge = 0,
+	.caps = PWRAP_CAP_RESET | PWRAP_CAP_DCM,
 	.init_reg_clock = pwrap_common_init_reg_clock,
 	.init_soc_specific = pwrap_mt8173_init_soc_specific,
 };
@@ -1509,7 +1505,7 @@ static int pwrap_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	if (wrp->master->has_bridge) {
+	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_BRIDGE)) {
 		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
 				"pwrap-bridge");
 		wrp->bridge_base = devm_ioremap_resource(wrp->dev, res);
-- 
2.12.5

[PATCH V4 4/8] soc: mediatek: pwrap: add int1_en_all flag

[argus.lin]

[-- Attachment #1.1: Type: text/html, Size: 2989 bytes --]

[-- Attachment #1.2: Type: text/plain, Size: 2063 bytes --]

From: Argus Lin <argus.lin@mediatek.com>

MT6797 support int1_en flag for starvation
and cmd_miss exception interrupt. We add
a new flag int1_en_all to check if we need
to enable interrupt source or not.

Signed-off-by: Argus Lin <argus.lin@mediatek.com>
---
 drivers/soc/mediatek/mtk-pmic-wrap.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index 911a8508f39f..a6366f147b79 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -687,6 +687,7 @@ struct pmic_wrapper_type {
 	enum pwrap_type type;
 	u32 arb_en_all;
 	u32 int_en_all;
+	u32 int1_en_all;
 	u32 spi_w;
 	u32 wdt_src;
 	/* Flags indicating the capability for the target pwrap */
@@ -1398,6 +1399,7 @@ static const struct pmic_wrapper_type pwrap_mt2701 = {
 	.type = PWRAP_MT2701,
 	.arb_en_all = 0x3f,
 	.int_en_all = ~(u32)(BIT(31) | BIT(2)),
+	.int1_en_all = 0,
 	.spi_w = PWRAP_MAN_CMD_SPI_WRITE_NEW,
 	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
 	.caps = PWRAP_CAP_RESET | PWRAP_CAP_DCM,
@@ -1410,6 +1412,7 @@ static const struct pmic_wrapper_type pwrap_mt7622 = {
 	.type = PWRAP_MT7622,
 	.arb_en_all = 0xff,
 	.int_en_all = ~(u32)BIT(31),
+	.int1_en_all = 0,
 	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
 	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
 	.caps = PWRAP_CAP_RESET | PWRAP_CAP_DCM,
@@ -1422,6 +1425,7 @@ static const struct pmic_wrapper_type pwrap_mt8135 = {
 	.type = PWRAP_MT8135,
 	.arb_en_all = 0x1ff,
 	.int_en_all = ~(u32)(BIT(31) | BIT(1)),
+	.int1_en_all = 0,
 	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
 	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
 	.caps = PWRAP_CAP_BRIDGE | PWRAP_CAP_RESET | PWRAP_CAP_DCM,
@@ -1434,6 +1438,7 @@ static const struct pmic_wrapper_type pwrap_mt8173 = {
 	.type = PWRAP_MT8173,
 	.arb_en_all = 0x3f,
 	.int_en_all = ~(u32)(BIT(31) | BIT(1)),
+	.int1_en_all = 0,
 	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
 	.wdt_src = PWRAP_WDT_SRC_MASK_NO_STAUPD,
 	.caps = PWRAP_CAP_RESET | PWRAP_CAP_DCM,
-- 
2.12.5

[PATCH V4 5/8] soc: mediatek: pwrap: add pwrap for mt6797 SoCs

[argus.lin]

[-- Attachment #1.1: Type: text/html, Size: 7548 bytes --]

[-- Attachment #1.2: Type: text/plain, Size: 6426 bytes --]

From: Argus Lin <argus.lin@mediatek.com>

mt6797 is a highly integrated SoCs, it uses mt6351 for power
management. We need to add pwrap support to access mt6351.
Pwrap of mt6797 support new feature include starvation and channel
request exception interrupt, dynamic starvation priority
adjustment mechanism.

Signed-off-by: Argus Lin <argus.lin@mediatek.com>
---
 drivers/soc/mediatek/mtk-pmic-wrap.c | 110 ++++++++++++++++++++++++++++++++---
 1 file changed, 102 insertions(+), 8 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index a6366f147b79..0d4a2dae6912 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -284,6 +284,12 @@ enum pwrap_regs {
 	PWRAP_DVFS_WDATA7,
 	PWRAP_SPMINF_STA,
 	PWRAP_CIPHER_EN,
+
+	/* MT6797 series regs */
+	PWRAP_INT1_EN,
+	PWRAP_INT1_FLG_RAW,
+	PWRAP_INT1_FLG,
+	PWRAP_INT1_CLR,
 };
 
 static int mt2701_regs[] = {
@@ -372,6 +378,43 @@ static int mt2701_regs[] = {
 	[PWRAP_ADC_RDATA_ADDR2] =	0x154,
 };
 
+static int mt6797_regs[] = {
+	[PWRAP_MUX_SEL] =		0x0,
+	[PWRAP_WRAP_EN] =		0x4,
+	[PWRAP_DIO_EN] =		0x8,
+	[PWRAP_SIDLY] =			0xC,
+	[PWRAP_RDDMY] =			0x10,
+	[PWRAP_CSHEXT_WRITE] =		0x18,
+	[PWRAP_CSHEXT_READ] =		0x1C,
+	[PWRAP_CSLEXT_START] =		0x20,
+	[PWRAP_CSLEXT_END] =		0x24,
+	[PWRAP_STAUPD_PRD] =		0x28,
+	[PWRAP_HARB_HPRIO] =		0x50,
+	[PWRAP_HIPRIO_ARB_EN] =		0x54,
+	[PWRAP_MAN_EN] =		0x60,
+	[PWRAP_MAN_CMD] =		0x64,
+	[PWRAP_WACS0_EN] =		0x70,
+	[PWRAP_WACS1_EN] =		0x84,
+	[PWRAP_WACS2_EN] =		0x98,
+	[PWRAP_INIT_DONE2] =		0x9C,
+	[PWRAP_WACS2_CMD] =		0xA0,
+	[PWRAP_WACS2_RDATA] =		0xA4,
+	[PWRAP_WACS2_VLDCLR] =		0xA8,
+	[PWRAP_INT_EN] =		0xC0,
+	[PWRAP_INT_FLG_RAW] =		0xC4,
+	[PWRAP_INT_FLG] =		0xC8,
+	[PWRAP_INT_CLR] =		0xCC,
+	[PWRAP_INT1_EN] =		0xD0,
+	[PWRAP_INT1_FLG_RAW] =		0xD4,
+	[PWRAP_INT1_FLG] =		0xD8,
+	[PWRAP_INT1_CLR] =		0xDC,
+	[PWRAP_TIMER_EN] =		0xF4,
+	[PWRAP_WDT_UNIT] =		0xFC,
+	[PWRAP_WDT_SRC_EN] =		0x100,
+	[PWRAP_DCM_EN] =		0x1CC,
+	[PWRAP_DCM_DBC_PRD] =		0x1D4,
+};
+
 static int mt7622_regs[] = {
 	[PWRAP_MUX_SEL] =		0x0,
 	[PWRAP_WRAP_EN] =		0x4,
@@ -647,6 +690,7 @@ enum pmic_type {
 
 enum pwrap_type {
 	PWRAP_MT2701,
+	PWRAP_MT6797,
 	PWRAP_MT7622,
 	PWRAP_MT8135,
 	PWRAP_MT8173,
@@ -1006,6 +1050,12 @@ static void pwrap_init_chip_select_ext(struct pmic_wrapper *wrp, u8 hext_write,
 static int pwrap_common_init_reg_clock(struct pmic_wrapper *wrp)
 {
 	switch (wrp->master->type) {
+	case PWRAP_MT6797:
+		pwrap_writel(wrp, 0x8, PWRAP_RDDMY);
+		pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_RDDMY_NO],
+			    0x8);
+		pwrap_init_chip_select_ext(wrp, 0x88, 0x55, 3, 0);
+		break;
 	case PWRAP_MT8173:
 		pwrap_init_chip_select_ext(wrp, 0, 4, 2, 2);
 		break;
@@ -1076,11 +1126,14 @@ static int pwrap_init_cipher(struct pmic_wrapper *wrp)
 		break;
 	case PWRAP_MT2701:
 	case PWRAP_MT8173:
+	case PWRAP_MT6797:
 		pwrap_writel(wrp, 1, PWRAP_CIPHER_EN);
 		break;
 	case PWRAP_MT7622:
 		pwrap_writel(wrp, 0, PWRAP_CIPHER_EN);
 		break;
+	default:
+		break;
 	}
 
 	/* Config cipher mode @PMIC */
@@ -1325,6 +1378,15 @@ static irqreturn_t pwrap_interrupt(int irqno, void *dev_id)
 
 	pwrap_writel(wrp, 0xffffffff, PWRAP_INT_CLR);
 
+	/* If we support INT1 interrupt, we also need to clear it */
+	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_INT1_EN)) {
+		rdata = pwrap_readl(wrp, PWRAP_INT1_FLG);
+
+		dev_err(wrp->dev, "unexpected interrupt int1=0x%x\n", rdata);
+
+		pwrap_writel(wrp, rdata, PWRAP_INT1_CLR);
+	}
+
 	return IRQ_HANDLED;
 }
 
@@ -1446,6 +1508,19 @@ static const struct pmic_wrapper_type pwrap_mt8173 = {
 	.init_soc_specific = pwrap_mt8173_init_soc_specific,
 };
 
+static const struct pmic_wrapper_type pwrap_mt6797 = {
+	.regs = mt6797_regs,
+	.type = PWRAP_MT6797,
+	.arb_en_all = 0x01fff,
+	.int_en_all = 0xffffffc6,
+	.int1_en_all = 0x0001ffff,
+	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
+	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
+	.caps = PWRAP_CAP_DCM | PWRAP_CAP_INT1_EN,
+	.init_reg_clock = pwrap_common_init_reg_clock,
+	.init_soc_specific = NULL,
+};
+
 static const struct of_device_id of_pwrap_match_tbl[] = {
 	{
 		.compatible = "mediatek,mt2701-pwrap",
@@ -1460,6 +1535,9 @@ static const struct of_device_id of_pwrap_match_tbl[] = {
 		.compatible = "mediatek,mt8173-pwrap",
 		.data = &pwrap_mt8173,
 	}, {
+		.compatible = "mediatek,mt6797-pwrap",
+		.data = &pwrap_mt6797,
+	}, {
 		/* sentinel */
 	}
 };
@@ -1503,11 +1581,13 @@ static int pwrap_probe(struct platform_device *pdev)
 	if (IS_ERR(wrp->base))
 		return PTR_ERR(wrp->base);
 
-	wrp->rstc = devm_reset_control_get(wrp->dev, "pwrap");
-	if (IS_ERR(wrp->rstc)) {
-		ret = PTR_ERR(wrp->rstc);
-		dev_dbg(wrp->dev, "cannot get pwrap reset: %d\n", ret);
-		return ret;
+	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_RESET)) {
+		wrp->rstc = devm_reset_control_get(wrp->dev, "pwrap");
+		if (IS_ERR(wrp->rstc)) {
+			ret = PTR_ERR(wrp->rstc);
+			dev_dbg(wrp->dev, "cannot get pwrap reset: %d\n", ret);
+			return ret;
+		}
 	}
 
 	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_BRIDGE)) {
@@ -1549,9 +1629,17 @@ static int pwrap_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_out1;
 
-	/* Enable internal dynamic clock */
-	pwrap_writel(wrp, 1, PWRAP_DCM_EN);
-	pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
+	/*
+	 * add dcm capability check
+	 */
+	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_DCM)) {
+		if (wrp->master->type == PWRAP_MT6797)
+			pwrap_writel(wrp, 3, PWRAP_DCM_EN);
+		else
+			pwrap_writel(wrp, 1, PWRAP_DCM_EN);
+
+		pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
+	}
 
 	/*
 	 * The PMIC could already be initialized by the bootloader.
@@ -1580,6 +1668,12 @@ static int pwrap_probe(struct platform_device *pdev)
 	pwrap_writel(wrp, wrp->master->wdt_src, PWRAP_WDT_SRC_EN);
 	pwrap_writel(wrp, 0x1, PWRAP_TIMER_EN);
 	pwrap_writel(wrp, wrp->master->int_en_all, PWRAP_INT_EN);
+	/*
+	 * We add INT1 interrupt to handle starvation and request exception
+	 * If we support it, we should enable them here.
+	 */
+	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_INT1_EN))
+		pwrap_writel(wrp, wrp->master->int1_en_all, PWRAP_INT1_EN);
 
 	irq = platform_get_irq(pdev, 0);
 	ret = devm_request_irq(wrp->dev, irq, pwrap_interrupt,
-- 
2.12.5

[PATCH V4 6/8] soc: mediatek: pwrap: fix cipher init

[argus.lin]

[-- Attachment #1.1: Type: text/html, Size: 2132 bytes --]

[-- Attachment #1.2: Type: text/plain, Size: 1150 bytes --]

From: Argus Lin <argus.lin@mediatek.com>

PWRAP_DEW_CIPHER_LOAD and PWRAP_DEW_CIPHER_START
only exist at PMIC_mt6397 register table. We delete them
and use wrp->slave->type for different chip's setting.

Fixes: 5ae48040aa47 ("soc: mediatek: PMIC wrap: add mt6323 slave support")
Signed-off-by: Argus Lin <argus.lin@mediatek.com>
---
 drivers/soc/mediatek/mtk-pmic-wrap.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index 0d4a2dae6912..285bfa76249f 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -1141,8 +1141,6 @@ static int pwrap_init_cipher(struct pmic_wrapper *wrp)
 	pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_CIPHER_SWRST], 0x0);
 	pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_CIPHER_KEY_SEL], 0x1);
 	pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_CIPHER_IV_SEL], 0x2);
-	pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_CIPHER_LOAD], 0x1);
-	pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_CIPHER_START], 0x1);
 
 	switch (wrp->slave->type) {
 	case PMIC_MT6397:
-- 
2.12.5

[PATCH V4 7/8] soc: mediatek: pwrap: add mt6351 for mt6797 SoCs

[argus.lin]

[-- Attachment #1.1: Type: text/html, Size: 3306 bytes --]

[-- Attachment #1.2: Type: text/plain, Size: 2332 bytes --]

From: Argus Lin <argus.lin@mediatek.com>

mt6351 is a new power management IC and it is
used for mt6797 SoCs. We need to add mt6351_regs for
pmic register mapping and pmic_mt6351 for
register accessing by regmap.

Signed-off-by: Argus Lin <argus.lin@mediatek.com>
---
 drivers/soc/mediatek/mtk-pmic-wrap.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index 285bfa76249f..26076900eee0 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -152,6 +152,21 @@ static const u32 mt6397_regs[] = {
 	[PWRAP_DEW_CIPHER_SWRST] =	0xbc24,
 };
 
+static const u32 mt6351_regs[] = {
+	[PWRAP_DEW_DIO_EN] =		0x02F2,
+	[PWRAP_DEW_READ_TEST] =		0x02F4,
+	[PWRAP_DEW_WRITE_TEST] =	0x02F6,
+	[PWRAP_DEW_CRC_EN] =		0x02FA,
+	[PWRAP_DEW_CRC_VAL] =		0x02FC,
+	[PWRAP_DEW_CIPHER_KEY_SEL] =	0x0300,
+	[PWRAP_DEW_CIPHER_IV_SEL] =	0x0302,
+	[PWRAP_DEW_CIPHER_EN] =		0x0304,
+	[PWRAP_DEW_CIPHER_RDY] =	0x0306,
+	[PWRAP_DEW_CIPHER_MODE] =	0x0308,
+	[PWRAP_DEW_CIPHER_SWRST] =	0x030A,
+	[PWRAP_DEW_RDDMY_NO] =		0x030C,
+};
+
 enum pwrap_regs {
 	PWRAP_MUX_SEL,
 	PWRAP_WRAP_EN,
@@ -684,6 +699,7 @@ static int mt8135_regs[] = {
 
 enum pmic_type {
 	PMIC_MT6323,
+	PMIC_MT6351,
 	PMIC_MT6380,
 	PMIC_MT6397,
 };
@@ -1150,6 +1166,7 @@ static int pwrap_init_cipher(struct pmic_wrapper *wrp)
 			    0x1);
 		break;
 	case PMIC_MT6323:
+	case PMIC_MT6351:
 		pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_CIPHER_EN],
 			    0x1);
 		break;
@@ -1435,6 +1452,15 @@ static const struct pwrap_slv_type pmic_mt6397 = {
 	.pwrap_write = pwrap_write16,
 };
 
+static const struct pwrap_slv_type pmic_mt6351 = {
+	.dew_regs = mt6351_regs,
+	.type = PMIC_MT6351,
+	.regmap = &pwrap_regmap_config16,
+	.caps = 0,
+	.pwrap_read = pwrap_read16,
+	.pwrap_write = pwrap_write16,
+};
+
 static const struct of_device_id of_slave_match_tbl[] = {
 	{
 		.compatible = "mediatek,mt6323",
@@ -1449,6 +1475,9 @@ static const struct of_device_id of_slave_match_tbl[] = {
 		.compatible = "mediatek,mt6397",
 		.data = &pmic_mt6397,
 	}, {
+		.compatible = "mediatek,mt6351",
+		.data = &pmic_mt6351,
+	}, {
 		/* sentinel */
 	}
 };
-- 
2.12.5

[PATCH V4 8/8] arm64: dts: mt6797: add pwrap support for mt6797

[argus.lin]

[-- Attachment #1.1: Type: text/html, Size: 2259 bytes --]

[-- Attachment #1.2: Type: text/plain, Size: 1225 bytes --]

From: Argus Lin <argus.lin@mediatek.com>

mt6797 is a highly integrated SoCs, and it uses
mt6351 as Power Management IC.
We need to add pwrap device to communicate with
mt6351 by SPI.
The base address of pwrap is 0x1000d000, and IRQ
number is 178. It also using fixed 26Mhz clock
as SPI CLK.

Signed-off-by: Argus Lin <argus.lin@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt6797.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt6797.dtsi b/arch/arm64/boot/dts/mediatek/mt6797.dtsi
index 4beaa71107d7..485546efc9bf 100644
--- a/arch/arm64/boot/dts/mediatek/mt6797.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt6797.dtsi
@@ -161,6 +161,20 @@
 		      <0 0x10220690 0 0x10>;
 	};
 
+	pwrap: pwrap@1000d000 {
+		compatible = "mediatek,mt6797-pwrap";
+		reg = <0 0x1000d000 0 0x1000>;
+		reg-names = "pwrap";
+		interrupts = <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&clk26m>, <&clk26m>;
+		clock-names = "spi", "wrap";
+
+		pmic: mt6351 {
+			compatible = "mediatek,mt6351";
+			interrupt-controller;
+		};
+	};
+
 	uart0: serial@11002000 {
 		compatible = "mediatek,mt6797-uart",
 			     "mediatek,mt6577-uart";
-- 
2.12.5

Re: [PATCH V4 2/8] soc: mediatek: pwrap: add caps flag for pwrap

[Matthias Brugger]

On 05/02/2018 11:21 AM, argus.lin@mediatek.com wrote:
> From: Argus Lin <argus.lin@mediatek.com>
> 
> We use caps to describe pwrap's capability, used
> to replace has_bridge flag for single meaning.
> 
> Signed-off-by: Argus Lin <argus.lin@mediatek.com>
> ---
>  drivers/soc/mediatek/mtk-pmic-wrap.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index e9e054a15b7d..3a25ff6e6907 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -76,6 +76,12 @@
>  #define PWRAP_SLV_CAP_SECURITY	BIT(2)
>  #define HAS_CAP(_c, _x)	(((_c) & (_x)) == (_x))
>  
> +/* Group of bits used for shown pwrap capability */
> +#define PWRAP_CAP_BRIDGE	BIT(0)
> +#define PWRAP_CAP_RESET		BIT(1)
> +#define PWRAP_CAP_DCM		BIT(2)
> +#define PWRAP_CAP_INT1_EN	BIT(3)
> +

You are missing the logic to handle the caps.
With out this logic, this patch is useless.

Regards,
Matthias

>  /* defines for slave device wrapper registers */
>  enum dew_regs {
>  	PWRAP_DEW_BASE,
> @@ -684,6 +690,8 @@ struct pmic_wrapper_type {
>  	u32 spi_w;
>  	u32 wdt_src;
>  	unsigned int has_bridge:1;
> +	/* Flags indicating the capability for the target pwrap */
> +	u8 caps;
>  	int (*init_reg_clock)(struct pmic_wrapper *wrp);
>  	int (*init_soc_specific)(struct pmic_wrapper *wrp);
>  };
> @@ -1394,6 +1402,7 @@ static const struct pmic_wrapper_type pwrap_mt2701 = {
>  	.spi_w = PWRAP_MAN_CMD_SPI_WRITE_NEW,
>  	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
>  	.has_bridge = 0,
> +	.caps = PWRAP_CAP_RESET | PWRAP_CAP_DCM,
>  	.init_reg_clock = pwrap_mt2701_init_reg_clock,
>  	.init_soc_specific = pwrap_mt2701_init_soc_specific,
>  };
> @@ -1406,6 +1415,7 @@ static const struct pmic_wrapper_type pwrap_mt7622 = {
>  	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
>  	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
>  	.has_bridge = 0,
> +	.caps = PWRAP_CAP_RESET | PWRAP_CAP_DCM,
>  	.init_reg_clock = pwrap_common_init_reg_clock,
>  	.init_soc_specific = pwrap_mt7622_init_soc_specific,
>  };
> @@ -1418,6 +1428,7 @@ static const struct pmic_wrapper_type pwrap_mt8135 = {
>  	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
>  	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
>  	.has_bridge = 1,
> +	.caps = PWRAP_CAP_BRIDGE | PWRAP_CAP_RESET | PWRAP_CAP_DCM,
>  	.init_reg_clock = pwrap_common_init_reg_clock,
>  	.init_soc_specific = pwrap_mt8135_init_soc_specific,
>  };
> -- 
> 2.12.5
> 
> ************* Email Confidentiality Notice ********************
> The information contained in this e-mail message (including any 
> attachments) may be confidential, proprietary, privileged, or otherwise
> exempt from disclosure under applicable laws. It is intended to be 
> conveyed only to the designated recipient(s). Any use, dissemination, 
> distribution, printing, retaining or copying of this e-mail (including its 
> attachments) by unintended recipient(s) is strictly prohibited and may 
> be unlawful. If you are not an intended recipient of this e-mail, or believe 
> that you have received this e-mail in error, please notify the sender 
> immediately (by replying to this e-mail), delete any and all copies of 
> this e-mail (including any attachments) from your system, and do not
> disclose the content of this e-mail to any other person. Thank you!
> 

[PATCH V4 2/8] soc: mediatek: pwrap: add caps flag for pwrap

[Matthias Brugger]

On 05/02/2018 11:21 AM, argus.lin at mediatek.com wrote:
> From: Argus Lin <argus.lin@mediatek.com>
> 
> We use caps to describe pwrap's capability, used
> to replace has_bridge flag for single meaning.
> 
> Signed-off-by: Argus Lin <argus.lin@mediatek.com>
> ---
>  drivers/soc/mediatek/mtk-pmic-wrap.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index e9e054a15b7d..3a25ff6e6907 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -76,6 +76,12 @@
>  #define PWRAP_SLV_CAP_SECURITY	BIT(2)
>  #define HAS_CAP(_c, _x)	(((_c) & (_x)) == (_x))
>  
> +/* Group of bits used for shown pwrap capability */
> +#define PWRAP_CAP_BRIDGE	BIT(0)
> +#define PWRAP_CAP_RESET		BIT(1)
> +#define PWRAP_CAP_DCM		BIT(2)
> +#define PWRAP_CAP_INT1_EN	BIT(3)
> +

You are missing the logic to handle the caps.
With out this logic, this patch is useless.

Regards,
Matthias

>  /* defines for slave device wrapper registers */
>  enum dew_regs {
>  	PWRAP_DEW_BASE,
> @@ -684,6 +690,8 @@ struct pmic_wrapper_type {
>  	u32 spi_w;
>  	u32 wdt_src;
>  	unsigned int has_bridge:1;
> +	/* Flags indicating the capability for the target pwrap */
> +	u8 caps;
>  	int (*init_reg_clock)(struct pmic_wrapper *wrp);
>  	int (*init_soc_specific)(struct pmic_wrapper *wrp);
>  };
> @@ -1394,6 +1402,7 @@ static const struct pmic_wrapper_type pwrap_mt2701 = {
>  	.spi_w = PWRAP_MAN_CMD_SPI_WRITE_NEW,
>  	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
>  	.has_bridge = 0,
> +	.caps = PWRAP_CAP_RESET | PWRAP_CAP_DCM,
>  	.init_reg_clock = pwrap_mt2701_init_reg_clock,
>  	.init_soc_specific = pwrap_mt2701_init_soc_specific,
>  };
> @@ -1406,6 +1415,7 @@ static const struct pmic_wrapper_type pwrap_mt7622 = {
>  	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
>  	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
>  	.has_bridge = 0,
> +	.caps = PWRAP_CAP_RESET | PWRAP_CAP_DCM,
>  	.init_reg_clock = pwrap_common_init_reg_clock,
>  	.init_soc_specific = pwrap_mt7622_init_soc_specific,
>  };
> @@ -1418,6 +1428,7 @@ static const struct pmic_wrapper_type pwrap_mt8135 = {
>  	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
>  	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
>  	.has_bridge = 1,
> +	.caps = PWRAP_CAP_BRIDGE | PWRAP_CAP_RESET | PWRAP_CAP_DCM,
>  	.init_reg_clock = pwrap_common_init_reg_clock,
>  	.init_soc_specific = pwrap_mt8135_init_soc_specific,
>  };
> -- 
> 2.12.5
> 
> ************* Email Confidentiality Notice ********************
> The information contained in this e-mail message (including any 
> attachments) may be confidential, proprietary, privileged, or otherwise
> exempt from disclosure under applicable laws. It is intended to be 
> conveyed only to the designated recipient(s). Any use, dissemination, 
> distribution, printing, retaining or copying of this e-mail (including its 
> attachments) by unintended recipient(s) is strictly prohibited and may 
> be unlawful. If you are not an intended recipient of this e-mail, or believe 
> that you have received this e-mail in error, please notify the sender 
> immediately (by replying to this e-mail), delete any and all copies of 
> this e-mail (including any attachments) from your system, and do not
> disclose the content of this e-mail to any other person. Thank you!
> 

Re: [PATCH V4 3/8] soc: mediatek: pwrap: remove has_bridge flag

[Matthias Brugger]

On 05/02/2018 11:21 AM, argus.lin@mediatek.com wrote:
> From: Argus Lin <argus.lin@mediatek.com>
> 
> We use new flag caps to replace has_bridge.
> Legacy chips support bridge use PWRAP_CAP_BRIDGE
> to explain such capability.
> 
> Signed-off-by: Argus Lin <argus.lin@mediatek.com>
> ---

Squash this one into 2/8.

Regards,
Matthias

>  drivers/soc/mediatek/mtk-pmic-wrap.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index 3a25ff6e6907..911a8508f39f 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -689,7 +689,6 @@ struct pmic_wrapper_type {
>  	u32 int_en_all;
>  	u32 spi_w;
>  	u32 wdt_src;
> -	unsigned int has_bridge:1;
>  	/* Flags indicating the capability for the target pwrap */
>  	u8 caps;
>  	int (*init_reg_clock)(struct pmic_wrapper *wrp);
> @@ -1306,7 +1305,7 @@ static int pwrap_init(struct pmic_wrapper *wrp)
>  	pwrap_writel(wrp, 1, PWRAP_INIT_DONE0);
>  	pwrap_writel(wrp, 1, PWRAP_INIT_DONE1);
>  
> -	if (wrp->master->has_bridge) {
> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_BRIDGE)) {
>  		writel(1, wrp->bridge_base + PWRAP_MT8135_BRIDGE_INIT_DONE3);
>  		writel(1, wrp->bridge_base + PWRAP_MT8135_BRIDGE_INIT_DONE4);
>  	}
> @@ -1401,7 +1400,6 @@ static const struct pmic_wrapper_type pwrap_mt2701 = {
>  	.int_en_all = ~(u32)(BIT(31) | BIT(2)),
>  	.spi_w = PWRAP_MAN_CMD_SPI_WRITE_NEW,
>  	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
> -	.has_bridge = 0,
>  	.caps = PWRAP_CAP_RESET | PWRAP_CAP_DCM,
>  	.init_reg_clock = pwrap_mt2701_init_reg_clock,
>  	.init_soc_specific = pwrap_mt2701_init_soc_specific,
> @@ -1414,7 +1412,6 @@ static const struct pmic_wrapper_type pwrap_mt7622 = {
>  	.int_en_all = ~(u32)BIT(31),
>  	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
>  	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
> -	.has_bridge = 0,
>  	.caps = PWRAP_CAP_RESET | PWRAP_CAP_DCM,
>  	.init_reg_clock = pwrap_common_init_reg_clock,
>  	.init_soc_specific = pwrap_mt7622_init_soc_specific,
> @@ -1427,7 +1424,6 @@ static const struct pmic_wrapper_type pwrap_mt8135 = {
>  	.int_en_all = ~(u32)(BIT(31) | BIT(1)),
>  	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
>  	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
> -	.has_bridge = 1,
>  	.caps = PWRAP_CAP_BRIDGE | PWRAP_CAP_RESET | PWRAP_CAP_DCM,
>  	.init_reg_clock = pwrap_common_init_reg_clock,
>  	.init_soc_specific = pwrap_mt8135_init_soc_specific,
> @@ -1440,7 +1436,7 @@ static const struct pmic_wrapper_type pwrap_mt8173 = {
>  	.int_en_all = ~(u32)(BIT(31) | BIT(1)),
>  	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
>  	.wdt_src = PWRAP_WDT_SRC_MASK_NO_STAUPD,
> -	.has_bridge = 0,
> +	.caps = PWRAP_CAP_RESET | PWRAP_CAP_DCM,
>  	.init_reg_clock = pwrap_common_init_reg_clock,
>  	.init_soc_specific = pwrap_mt8173_init_soc_specific,
>  };
> @@ -1509,7 +1505,7 @@ static int pwrap_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	if (wrp->master->has_bridge) {
> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_BRIDGE)) {
>  		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
>  				"pwrap-bridge");
>  		wrp->bridge_base = devm_ioremap_resource(wrp->dev, res);
> -- 
> 2.12.5
> 
> ************* Email Confidentiality Notice ********************
> The information contained in this e-mail message (including any 
> attachments) may be confidential, proprietary, privileged, or otherwise
> exempt from disclosure under applicable laws. It is intended to be 
> conveyed only to the designated recipient(s). Any use, dissemination, 
> distribution, printing, retaining or copying of this e-mail (including its 
> attachments) by unintended recipient(s) is strictly prohibited and may 
> be unlawful. If you are not an intended recipient of this e-mail, or believe 
> that you have received this e-mail in error, please notify the sender 
> immediately (by replying to this e-mail), delete any and all copies of 
> this e-mail (including any attachments) from your system, and do not
> disclose the content of this e-mail to any other person. Thank you!
> 

[PATCH V4 3/8] soc: mediatek: pwrap: remove has_bridge flag

[Matthias Brugger]

On 05/02/2018 11:21 AM, argus.lin at mediatek.com wrote:
> From: Argus Lin <argus.lin@mediatek.com>
> 
> We use new flag caps to replace has_bridge.
> Legacy chips support bridge use PWRAP_CAP_BRIDGE
> to explain such capability.
> 
> Signed-off-by: Argus Lin <argus.lin@mediatek.com>
> ---

Squash this one into 2/8.

Regards,
Matthias

>  drivers/soc/mediatek/mtk-pmic-wrap.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index 3a25ff6e6907..911a8508f39f 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -689,7 +689,6 @@ struct pmic_wrapper_type {
>  	u32 int_en_all;
>  	u32 spi_w;
>  	u32 wdt_src;
> -	unsigned int has_bridge:1;
>  	/* Flags indicating the capability for the target pwrap */
>  	u8 caps;
>  	int (*init_reg_clock)(struct pmic_wrapper *wrp);
> @@ -1306,7 +1305,7 @@ static int pwrap_init(struct pmic_wrapper *wrp)
>  	pwrap_writel(wrp, 1, PWRAP_INIT_DONE0);
>  	pwrap_writel(wrp, 1, PWRAP_INIT_DONE1);
>  
> -	if (wrp->master->has_bridge) {
> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_BRIDGE)) {
>  		writel(1, wrp->bridge_base + PWRAP_MT8135_BRIDGE_INIT_DONE3);
>  		writel(1, wrp->bridge_base + PWRAP_MT8135_BRIDGE_INIT_DONE4);
>  	}
> @@ -1401,7 +1400,6 @@ static const struct pmic_wrapper_type pwrap_mt2701 = {
>  	.int_en_all = ~(u32)(BIT(31) | BIT(2)),
>  	.spi_w = PWRAP_MAN_CMD_SPI_WRITE_NEW,
>  	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
> -	.has_bridge = 0,
>  	.caps = PWRAP_CAP_RESET | PWRAP_CAP_DCM,
>  	.init_reg_clock = pwrap_mt2701_init_reg_clock,
>  	.init_soc_specific = pwrap_mt2701_init_soc_specific,
> @@ -1414,7 +1412,6 @@ static const struct pmic_wrapper_type pwrap_mt7622 = {
>  	.int_en_all = ~(u32)BIT(31),
>  	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
>  	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
> -	.has_bridge = 0,
>  	.caps = PWRAP_CAP_RESET | PWRAP_CAP_DCM,
>  	.init_reg_clock = pwrap_common_init_reg_clock,
>  	.init_soc_specific = pwrap_mt7622_init_soc_specific,
> @@ -1427,7 +1424,6 @@ static const struct pmic_wrapper_type pwrap_mt8135 = {
>  	.int_en_all = ~(u32)(BIT(31) | BIT(1)),
>  	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
>  	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
> -	.has_bridge = 1,
>  	.caps = PWRAP_CAP_BRIDGE | PWRAP_CAP_RESET | PWRAP_CAP_DCM,
>  	.init_reg_clock = pwrap_common_init_reg_clock,
>  	.init_soc_specific = pwrap_mt8135_init_soc_specific,
> @@ -1440,7 +1436,7 @@ static const struct pmic_wrapper_type pwrap_mt8173 = {
>  	.int_en_all = ~(u32)(BIT(31) | BIT(1)),
>  	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
>  	.wdt_src = PWRAP_WDT_SRC_MASK_NO_STAUPD,
> -	.has_bridge = 0,
> +	.caps = PWRAP_CAP_RESET | PWRAP_CAP_DCM,
>  	.init_reg_clock = pwrap_common_init_reg_clock,
>  	.init_soc_specific = pwrap_mt8173_init_soc_specific,
>  };
> @@ -1509,7 +1505,7 @@ static int pwrap_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	if (wrp->master->has_bridge) {
> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_BRIDGE)) {
>  		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
>  				"pwrap-bridge");
>  		wrp->bridge_base = devm_ioremap_resource(wrp->dev, res);
> -- 
> 2.12.5
> 
> ************* Email Confidentiality Notice ********************
> The information contained in this e-mail message (including any 
> attachments) may be confidential, proprietary, privileged, or otherwise
> exempt from disclosure under applicable laws. It is intended to be 
> conveyed only to the designated recipient(s). Any use, dissemination, 
> distribution, printing, retaining or copying of this e-mail (including its 
> attachments) by unintended recipient(s) is strictly prohibited and may 
> be unlawful. If you are not an intended recipient of this e-mail, or believe 
> that you have received this e-mail in error, please notify the sender 
> immediately (by replying to this e-mail), delete any and all copies of 
> this e-mail (including any attachments) from your system, and do not
> disclose the content of this e-mail to any other person. Thank you!
> 

Re: [PATCH V4 4/8] soc: mediatek: pwrap: add int1_en_all flag

[Matthias Brugger]

On 05/02/2018 11:21 AM, argus.lin@mediatek.com wrote:
> From: Argus Lin <argus.lin@mediatek.com>
> 
> MT6797 support int1_en flag for starvation
> and cmd_miss exception interrupt. We add
> a new flag int1_en_all to check if we need
> to enable interrupt source or not.
> 
> Signed-off-by: Argus Lin <argus.lin@mediatek.com>
> ---
>  drivers/soc/mediatek/mtk-pmic-wrap.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
Same as 2/8. We need the logic that uses this flag in the same patch not in a
later one.

> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index 911a8508f39f..a6366f147b79 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -687,6 +687,7 @@ struct pmic_wrapper_type {
>  	enum pwrap_type type;
>  	u32 arb_en_all;
>  	u32 int_en_all;
> +	u32 int1_en_all;
>  	u32 spi_w;
>  	u32 wdt_src;
>  	/* Flags indicating the capability for the target pwrap */
> @@ -1398,6 +1399,7 @@ static const struct pmic_wrapper_type pwrap_mt2701 = {
>  	.type = PWRAP_MT2701,
>  	.arb_en_all = 0x3f,
>  	.int_en_all = ~(u32)(BIT(31) | BIT(2)),
> +	.int1_en_all = 0,
>  	.spi_w = PWRAP_MAN_CMD_SPI_WRITE_NEW,
>  	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
>  	.caps = PWRAP_CAP_RESET | PWRAP_CAP_DCM,
> @@ -1410,6 +1412,7 @@ static const struct pmic_wrapper_type pwrap_mt7622 = {
>  	.type = PWRAP_MT7622,
>  	.arb_en_all = 0xff,
>  	.int_en_all = ~(u32)BIT(31),
> +	.int1_en_all = 0,
>  	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
>  	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
>  	.caps = PWRAP_CAP_RESET | PWRAP_CAP_DCM,
> @@ -1422,6 +1425,7 @@ static const struct pmic_wrapper_type pwrap_mt8135 = {
>  	.type = PWRAP_MT8135,
>  	.arb_en_all = 0x1ff,
>  	.int_en_all = ~(u32)(BIT(31) | BIT(1)),
> +	.int1_en_all = 0,
>  	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
>  	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
>  	.caps = PWRAP_CAP_BRIDGE | PWRAP_CAP_RESET | PWRAP_CAP_DCM,
> @@ -1434,6 +1438,7 @@ static const struct pmic_wrapper_type pwrap_mt8173 = {
>  	.type = PWRAP_MT8173,
>  	.arb_en_all = 0x3f,
>  	.int_en_all = ~(u32)(BIT(31) | BIT(1)),
> +	.int1_en_all = 0,
>  	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
>  	.wdt_src = PWRAP_WDT_SRC_MASK_NO_STAUPD,
>  	.caps = PWRAP_CAP_RESET | PWRAP_CAP_DCM,
> -- 
> 2.12.5
> 
> ************* Email Confidentiality Notice ********************
> The information contained in this e-mail message (including any 
> attachments) may be confidential, proprietary, privileged, or otherwise
> exempt from disclosure under applicable laws. It is intended to be 
> conveyed only to the designated recipient(s). Any use, dissemination, 
> distribution, printing, retaining or copying of this e-mail (including its 
> attachments) by unintended recipient(s) is strictly prohibited and may 
> be unlawful. If you are not an intended recipient of this e-mail, or believe 
> that you have received this e-mail in error, please notify the sender 
> immediately (by replying to this e-mail), delete any and all copies of 
> this e-mail (including any attachments) from your system, and do not
> disclose the content of this e-mail to any other person. Thank you!
> 

[PATCH V4 4/8] soc: mediatek: pwrap: add int1_en_all flag

[Matthias Brugger]

On 05/02/2018 11:21 AM, argus.lin at mediatek.com wrote:
> From: Argus Lin <argus.lin@mediatek.com>
> 
> MT6797 support int1_en flag for starvation
> and cmd_miss exception interrupt. We add
> a new flag int1_en_all to check if we need
> to enable interrupt source or not.
> 
> Signed-off-by: Argus Lin <argus.lin@mediatek.com>
> ---
>  drivers/soc/mediatek/mtk-pmic-wrap.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
Same as 2/8. We need the logic that uses this flag in the same patch not in a
later one.

> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index 911a8508f39f..a6366f147b79 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -687,6 +687,7 @@ struct pmic_wrapper_type {
>  	enum pwrap_type type;
>  	u32 arb_en_all;
>  	u32 int_en_all;
> +	u32 int1_en_all;
>  	u32 spi_w;
>  	u32 wdt_src;
>  	/* Flags indicating the capability for the target pwrap */
> @@ -1398,6 +1399,7 @@ static const struct pmic_wrapper_type pwrap_mt2701 = {
>  	.type = PWRAP_MT2701,
>  	.arb_en_all = 0x3f,
>  	.int_en_all = ~(u32)(BIT(31) | BIT(2)),
> +	.int1_en_all = 0,
>  	.spi_w = PWRAP_MAN_CMD_SPI_WRITE_NEW,
>  	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
>  	.caps = PWRAP_CAP_RESET | PWRAP_CAP_DCM,
> @@ -1410,6 +1412,7 @@ static const struct pmic_wrapper_type pwrap_mt7622 = {
>  	.type = PWRAP_MT7622,
>  	.arb_en_all = 0xff,
>  	.int_en_all = ~(u32)BIT(31),
> +	.int1_en_all = 0,
>  	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
>  	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
>  	.caps = PWRAP_CAP_RESET | PWRAP_CAP_DCM,
> @@ -1422,6 +1425,7 @@ static const struct pmic_wrapper_type pwrap_mt8135 = {
>  	.type = PWRAP_MT8135,
>  	.arb_en_all = 0x1ff,
>  	.int_en_all = ~(u32)(BIT(31) | BIT(1)),
> +	.int1_en_all = 0,
>  	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
>  	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
>  	.caps = PWRAP_CAP_BRIDGE | PWRAP_CAP_RESET | PWRAP_CAP_DCM,
> @@ -1434,6 +1438,7 @@ static const struct pmic_wrapper_type pwrap_mt8173 = {
>  	.type = PWRAP_MT8173,
>  	.arb_en_all = 0x3f,
>  	.int_en_all = ~(u32)(BIT(31) | BIT(1)),
> +	.int1_en_all = 0,
>  	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
>  	.wdt_src = PWRAP_WDT_SRC_MASK_NO_STAUPD,
>  	.caps = PWRAP_CAP_RESET | PWRAP_CAP_DCM,
> -- 
> 2.12.5
> 
> ************* Email Confidentiality Notice ********************
> The information contained in this e-mail message (including any 
> attachments) may be confidential, proprietary, privileged, or otherwise
> exempt from disclosure under applicable laws. It is intended to be 
> conveyed only to the designated recipient(s). Any use, dissemination, 
> distribution, printing, retaining or copying of this e-mail (including its 
> attachments) by unintended recipient(s) is strictly prohibited and may 
> be unlawful. If you are not an intended recipient of this e-mail, or believe 
> that you have received this e-mail in error, please notify the sender 
> immediately (by replying to this e-mail), delete any and all copies of 
> this e-mail (including any attachments) from your system, and do not
> disclose the content of this e-mail to any other person. Thank you!
> 

Re: [PATCH V4 5/8] soc: mediatek: pwrap: add pwrap for mt6797 SoCs

[Matthias Brugger]

On 05/02/2018 11:21 AM, argus.lin@mediatek.com wrote:
> From: Argus Lin <argus.lin@mediatek.com>
> 
> mt6797 is a highly integrated SoCs, it uses mt6351 for power
> management. We need to add pwrap support to access mt6351.
> Pwrap of mt6797 support new feature include starvation and channel
> request exception interrupt, dynamic starvation priority
> adjustment mechanism.
> 
> Signed-off-by: Argus Lin <argus.lin@mediatek.com>
> ---
>  drivers/soc/mediatek/mtk-pmic-wrap.c | 110 ++++++++++++++++++++++++++++++++---
>  1 file changed, 102 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index a6366f147b79..0d4a2dae6912 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c

[...]

> @@ -1076,11 +1126,14 @@ static int pwrap_init_cipher(struct pmic_wrapper *wrp)
>  		break;
>  	case PWRAP_MT2701:
>  	case PWRAP_MT8173:
> +	case PWRAP_MT6797:
>  		pwrap_writel(wrp, 1, PWRAP_CIPHER_EN);
>  		break;
>  	case PWRAP_MT7622:
>  		pwrap_writel(wrp, 0, PWRAP_CIPHER_EN);
>  		break;
> +	default:
> +		break;

Why do we need that now?

>  	}
>  
>  	/* Config cipher mode @PMIC */
> @@ -1325,6 +1378,15 @@ static irqreturn_t pwrap_interrupt(int irqno, void *dev_id)
>  
>  	pwrap_writel(wrp, 0xffffffff, PWRAP_INT_CLR);
>  
> +	/* If we support INT1 interrupt, we also need to clear it */
> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_INT1_EN)) {
> +		rdata = pwrap_readl(wrp, PWRAP_INT1_FLG);
> +
> +		dev_err(wrp->dev, "unexpected interrupt int1=0x%x\n", rdata);
> +
> +		pwrap_writel(wrp, rdata, PWRAP_INT1_CLR);
> +	}
> +

This should go in 4/8. You will need to add PWRAP_INT1_FLG to the enum
pwrap_regs as well.

>  	return IRQ_HANDLED;
>  }
>  
> @@ -1446,6 +1508,19 @@ static const struct pmic_wrapper_type pwrap_mt8173 = {
>  	.init_soc_specific = pwrap_mt8173_init_soc_specific,
>  };
>  
> +static const struct pmic_wrapper_type pwrap_mt6797 = {
> +	.regs = mt6797_regs,
> +	.type = PWRAP_MT6797,
> +	.arb_en_all = 0x01fff,
> +	.int_en_all = 0xffffffc6,
> +	.int1_en_all = 0x0001ffff,
> +	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
> +	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
> +	.caps = PWRAP_CAP_DCM | PWRAP_CAP_INT1_EN,
> +	.init_reg_clock = pwrap_common_init_reg_clock,
> +	.init_soc_specific = NULL,
> +};
> +
>  static const struct of_device_id of_pwrap_match_tbl[] = {
>  	{
>  		.compatible = "mediatek,mt2701-pwrap",
> @@ -1460,6 +1535,9 @@ static const struct of_device_id of_pwrap_match_tbl[] = {
>  		.compatible = "mediatek,mt8173-pwrap",
>  		.data = &pwrap_mt8173,
>  	}, {
> +		.compatible = "mediatek,mt6797-pwrap",
> +		.data = &pwrap_mt6797,
> +	}, {
>  		/* sentinel */
>  	}
>  };
> @@ -1503,11 +1581,13 @@ static int pwrap_probe(struct platform_device *pdev)
>  	if (IS_ERR(wrp->base))
>  		return PTR_ERR(wrp->base);
>  
> -	wrp->rstc = devm_reset_control_get(wrp->dev, "pwrap");
> -	if (IS_ERR(wrp->rstc)) {
> -		ret = PTR_ERR(wrp->rstc);
> -		dev_dbg(wrp->dev, "cannot get pwrap reset: %d\n", ret);
> -		return ret;
> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_RESET)) {
> +		wrp->rstc = devm_reset_control_get(wrp->dev, "pwrap");
> +		if (IS_ERR(wrp->rstc)) {
> +			ret = PTR_ERR(wrp->rstc);
> +			dev_dbg(wrp->dev, "cannot get pwrap reset: %d\n", ret);
> +			return ret;
> +		}

This goes into 2/8.

>  	}
>  
>  	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_BRIDGE)) {
> @@ -1549,9 +1629,17 @@ static int pwrap_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err_out1;
>  
> -	/* Enable internal dynamic clock */
> -	pwrap_writel(wrp, 1, PWRAP_DCM_EN);
> -	pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
> +	/*
> +	 * add dcm capability check
> +	 */
> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_DCM)) {

Into 2/8 please.

> +		if (wrp->master->type == PWRAP_MT6797)
> +			pwrap_writel(wrp, 3, PWRAP_DCM_EN);
> +		else
> +			pwrap_writel(wrp, 1, PWRAP_DCM_EN);

The if statement is ok here, but it should change the if of the caps check
introduced in 2/8. Does this make sense?

> +
> +		pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
> +	}

Into 2/8 please.

>  
>  	/*
>  	 * The PMIC could already be initialized by the bootloader.
> @@ -1580,6 +1668,12 @@ static int pwrap_probe(struct platform_device *pdev)
>  	pwrap_writel(wrp, wrp->master->wdt_src, PWRAP_WDT_SRC_EN);
>  	pwrap_writel(wrp, 0x1, PWRAP_TIMER_EN);
>  	pwrap_writel(wrp, wrp->master->int_en_all, PWRAP_INT_EN);
> +	/*
> +	 * We add INT1 interrupt to handle starvation and request exception
> +	 * If we support it, we should enable them here.
> +	 */
> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_INT1_EN))
> +		pwrap_writel(wrp, wrp->master->int1_en_all, PWRAP_INT1_EN);

Into 4/8 please.

Best regards,
Matthias

[PATCH V4 5/8] soc: mediatek: pwrap: add pwrap for mt6797 SoCs

[Matthias Brugger]

On 05/02/2018 11:21 AM, argus.lin at mediatek.com wrote:
> From: Argus Lin <argus.lin@mediatek.com>
> 
> mt6797 is a highly integrated SoCs, it uses mt6351 for power
> management. We need to add pwrap support to access mt6351.
> Pwrap of mt6797 support new feature include starvation and channel
> request exception interrupt, dynamic starvation priority
> adjustment mechanism.
> 
> Signed-off-by: Argus Lin <argus.lin@mediatek.com>
> ---
>  drivers/soc/mediatek/mtk-pmic-wrap.c | 110 ++++++++++++++++++++++++++++++++---
>  1 file changed, 102 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index a6366f147b79..0d4a2dae6912 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c

[...]

> @@ -1076,11 +1126,14 @@ static int pwrap_init_cipher(struct pmic_wrapper *wrp)
>  		break;
>  	case PWRAP_MT2701:
>  	case PWRAP_MT8173:
> +	case PWRAP_MT6797:
>  		pwrap_writel(wrp, 1, PWRAP_CIPHER_EN);
>  		break;
>  	case PWRAP_MT7622:
>  		pwrap_writel(wrp, 0, PWRAP_CIPHER_EN);
>  		break;
> +	default:
> +		break;

Why do we need that now?

>  	}
>  
>  	/* Config cipher mode @PMIC */
> @@ -1325,6 +1378,15 @@ static irqreturn_t pwrap_interrupt(int irqno, void *dev_id)
>  
>  	pwrap_writel(wrp, 0xffffffff, PWRAP_INT_CLR);
>  
> +	/* If we support INT1 interrupt, we also need to clear it */
> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_INT1_EN)) {
> +		rdata = pwrap_readl(wrp, PWRAP_INT1_FLG);
> +
> +		dev_err(wrp->dev, "unexpected interrupt int1=0x%x\n", rdata);
> +
> +		pwrap_writel(wrp, rdata, PWRAP_INT1_CLR);
> +	}
> +

This should go in 4/8. You will need to add PWRAP_INT1_FLG to the enum
pwrap_regs as well.

>  	return IRQ_HANDLED;
>  }
>  
> @@ -1446,6 +1508,19 @@ static const struct pmic_wrapper_type pwrap_mt8173 = {
>  	.init_soc_specific = pwrap_mt8173_init_soc_specific,
>  };
>  
> +static const struct pmic_wrapper_type pwrap_mt6797 = {
> +	.regs = mt6797_regs,
> +	.type = PWRAP_MT6797,
> +	.arb_en_all = 0x01fff,
> +	.int_en_all = 0xffffffc6,
> +	.int1_en_all = 0x0001ffff,
> +	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
> +	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
> +	.caps = PWRAP_CAP_DCM | PWRAP_CAP_INT1_EN,
> +	.init_reg_clock = pwrap_common_init_reg_clock,
> +	.init_soc_specific = NULL,
> +};
> +
>  static const struct of_device_id of_pwrap_match_tbl[] = {
>  	{
>  		.compatible = "mediatek,mt2701-pwrap",
> @@ -1460,6 +1535,9 @@ static const struct of_device_id of_pwrap_match_tbl[] = {
>  		.compatible = "mediatek,mt8173-pwrap",
>  		.data = &pwrap_mt8173,
>  	}, {
> +		.compatible = "mediatek,mt6797-pwrap",
> +		.data = &pwrap_mt6797,
> +	}, {
>  		/* sentinel */
>  	}
>  };
> @@ -1503,11 +1581,13 @@ static int pwrap_probe(struct platform_device *pdev)
>  	if (IS_ERR(wrp->base))
>  		return PTR_ERR(wrp->base);
>  
> -	wrp->rstc = devm_reset_control_get(wrp->dev, "pwrap");
> -	if (IS_ERR(wrp->rstc)) {
> -		ret = PTR_ERR(wrp->rstc);
> -		dev_dbg(wrp->dev, "cannot get pwrap reset: %d\n", ret);
> -		return ret;
> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_RESET)) {
> +		wrp->rstc = devm_reset_control_get(wrp->dev, "pwrap");
> +		if (IS_ERR(wrp->rstc)) {
> +			ret = PTR_ERR(wrp->rstc);
> +			dev_dbg(wrp->dev, "cannot get pwrap reset: %d\n", ret);
> +			return ret;
> +		}

This goes into 2/8.

>  	}
>  
>  	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_BRIDGE)) {
> @@ -1549,9 +1629,17 @@ static int pwrap_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err_out1;
>  
> -	/* Enable internal dynamic clock */
> -	pwrap_writel(wrp, 1, PWRAP_DCM_EN);
> -	pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
> +	/*
> +	 * add dcm capability check
> +	 */
> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_DCM)) {

Into 2/8 please.

> +		if (wrp->master->type == PWRAP_MT6797)
> +			pwrap_writel(wrp, 3, PWRAP_DCM_EN);
> +		else
> +			pwrap_writel(wrp, 1, PWRAP_DCM_EN);

The if statement is ok here, but it should change the if of the caps check
introduced in 2/8. Does this make sense?

> +
> +		pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
> +	}

Into 2/8 please.

>  
>  	/*
>  	 * The PMIC could already be initialized by the bootloader.
> @@ -1580,6 +1668,12 @@ static int pwrap_probe(struct platform_device *pdev)
>  	pwrap_writel(wrp, wrp->master->wdt_src, PWRAP_WDT_SRC_EN);
>  	pwrap_writel(wrp, 0x1, PWRAP_TIMER_EN);
>  	pwrap_writel(wrp, wrp->master->int_en_all, PWRAP_INT_EN);
> +	/*
> +	 * We add INT1 interrupt to handle starvation and request exception
> +	 * If we support it, we should enable them here.
> +	 */
> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_INT1_EN))
> +		pwrap_writel(wrp, wrp->master->int1_en_all, PWRAP_INT1_EN);

Into 4/8 please.

Best regards,
Matthias

Re: [PATCH V4 1/8] dt-bindings: pwrap: mediatek: add MT6351 PMIC support for MT6797

[Sean Wang]

On Wed, 2018-05-02 at 17:21 +0800, argus.lin@mediatek.com wrote:
> From: Argus Lin <argus.lin@mediatek.com>
> 
> We add MT6351 PMIC support for MT6797 SoCs.
> 
> Signed-off-by: Argus Lin <argus.lin@mediatek.com>
> ---
>  Documentation/devicetree/bindings/soc/mediatek/pwrap.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/soc/mediatek/pwrap.txt b/Documentation/devicetree/bindings/soc/mediatek/pwrap.txt
> index bf80e3f96f8c..f9987c30f0d5 100644
> --- a/Documentation/devicetree/bindings/soc/mediatek/pwrap.txt
> +++ b/Documentation/devicetree/bindings/soc/mediatek/pwrap.txt
> @@ -19,6 +19,7 @@ IP Pairing
>  Required properties in pwrap device node.
>  - compatible:
>  	"mediatek,mt2701-pwrap" for MT2701/7623 SoCs
> +	"mediatek,mt6797-pwrap" for MT6797 SoCs
>  	"mediatek,mt7622-pwrap" for MT7622 SoCs
>  	"mediatek,mt8135-pwrap" for MT8135 SoCs
>  	"mediatek,mt8173-pwrap" for MT8173 SoCs

The content doesn't describe anything about MT6351, which seems to be
inconsistent with its title

Re: [PATCH V4 1/8] dt-bindings: pwrap: mediatek: add MT6351 PMIC support for MT6797

[Sean Wang]

On Wed, 2018-05-02 at 17:21 +0800, argus.lin@mediatek.com wrote:
> From: Argus Lin <argus.lin@mediatek.com>
> 
> We add MT6351 PMIC support for MT6797 SoCs.
> 
> Signed-off-by: Argus Lin <argus.lin@mediatek.com>
> ---
>  Documentation/devicetree/bindings/soc/mediatek/pwrap.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/soc/mediatek/pwrap.txt b/Documentation/devicetree/bindings/soc/mediatek/pwrap.txt
> index bf80e3f96f8c..f9987c30f0d5 100644
> --- a/Documentation/devicetree/bindings/soc/mediatek/pwrap.txt
> +++ b/Documentation/devicetree/bindings/soc/mediatek/pwrap.txt
> @@ -19,6 +19,7 @@ IP Pairing
>  Required properties in pwrap device node.
>  - compatible:
>  	"mediatek,mt2701-pwrap" for MT2701/7623 SoCs
> +	"mediatek,mt6797-pwrap" for MT6797 SoCs
>  	"mediatek,mt7622-pwrap" for MT7622 SoCs
>  	"mediatek,mt8135-pwrap" for MT8135 SoCs
>  	"mediatek,mt8173-pwrap" for MT8173 SoCs

The content doesn't describe anything about MT6351, which seems to be
inconsistent with its title

[PATCH V4 1/8] dt-bindings: pwrap: mediatek: add MT6351 PMIC support for MT6797

[Sean Wang]

On Wed, 2018-05-02 at 17:21 +0800, argus.lin at mediatek.com wrote:
> From: Argus Lin <argus.lin@mediatek.com>
> 
> We add MT6351 PMIC support for MT6797 SoCs.
> 
> Signed-off-by: Argus Lin <argus.lin@mediatek.com>
> ---
>  Documentation/devicetree/bindings/soc/mediatek/pwrap.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/soc/mediatek/pwrap.txt b/Documentation/devicetree/bindings/soc/mediatek/pwrap.txt
> index bf80e3f96f8c..f9987c30f0d5 100644
> --- a/Documentation/devicetree/bindings/soc/mediatek/pwrap.txt
> +++ b/Documentation/devicetree/bindings/soc/mediatek/pwrap.txt
> @@ -19,6 +19,7 @@ IP Pairing
>  Required properties in pwrap device node.
>  - compatible:
>  	"mediatek,mt2701-pwrap" for MT2701/7623 SoCs
> +	"mediatek,mt6797-pwrap" for MT6797 SoCs
>  	"mediatek,mt7622-pwrap" for MT7622 SoCs
>  	"mediatek,mt8135-pwrap" for MT8135 SoCs
>  	"mediatek,mt8173-pwrap" for MT8173 SoCs

The content doesn't describe anything about MT6351, which seems to be
inconsistent with its title

Re: [PATCH V4 8/8] arm64: dts: mt6797: add pwrap support for mt6797

[Sean Wang]

On Wed, 2018-05-02 at 17:21 +0800, argus.lin@mediatek.com wrote:
> From: Argus Lin <argus.lin@mediatek.com>
> 
> mt6797 is a highly integrated SoCs, and it uses
> mt6351 as Power Management IC.
> We need to add pwrap device to communicate with
> mt6351 by SPI.
> The base address of pwrap is 0x1000d000, and IRQ
> number is 178. It also using fixed 26Mhz clock
> as SPI CLK.
> 
> Signed-off-by: Argus Lin <argus.lin@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt6797.dtsi | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt6797.dtsi b/arch/arm64/boot/dts/mediatek/mt6797.dtsi
> index 4beaa71107d7..485546efc9bf 100644
> --- a/arch/arm64/boot/dts/mediatek/mt6797.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt6797.dtsi
> @@ -161,6 +161,20 @@
>  		      <0 0x10220690 0 0x10>;
>  	};
>  
> +	pwrap: pwrap@1000d000 {
> +		compatible = "mediatek,mt6797-pwrap";
> +		reg = <0 0x1000d000 0 0x1000>;
> +		reg-names = "pwrap";
> +		interrupts = <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&clk26m>, <&clk26m>;
> +		clock-names = "spi", "wrap";
> +
> +		pmic: mt6351 {
> +			compatible = "mediatek,mt6351";
> +			interrupt-controller;

the child node can't be added until MT6351 support is added to
dt-binding

> +		};
> +	};
> +
>  	uart0: serial@11002000 {
>  		compatible = "mediatek,mt6797-uart",
>  			     "mediatek,mt6577-uart";

Re: [PATCH V4 8/8] arm64: dts: mt6797: add pwrap support for mt6797

[Sean Wang]

On Wed, 2018-05-02 at 17:21 +0800, argus.lin@mediatek.com wrote:
> From: Argus Lin <argus.lin@mediatek.com>
> 
> mt6797 is a highly integrated SoCs, and it uses
> mt6351 as Power Management IC.
> We need to add pwrap device to communicate with
> mt6351 by SPI.
> The base address of pwrap is 0x1000d000, and IRQ
> number is 178. It also using fixed 26Mhz clock
> as SPI CLK.
> 
> Signed-off-by: Argus Lin <argus.lin@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt6797.dtsi | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt6797.dtsi b/arch/arm64/boot/dts/mediatek/mt6797.dtsi
> index 4beaa71107d7..485546efc9bf 100644
> --- a/arch/arm64/boot/dts/mediatek/mt6797.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt6797.dtsi
> @@ -161,6 +161,20 @@
>  		      <0 0x10220690 0 0x10>;
>  	};
>  
> +	pwrap: pwrap@1000d000 {
> +		compatible = "mediatek,mt6797-pwrap";
> +		reg = <0 0x1000d000 0 0x1000>;
> +		reg-names = "pwrap";
> +		interrupts = <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&clk26m>, <&clk26m>;
> +		clock-names = "spi", "wrap";
> +
> +		pmic: mt6351 {
> +			compatible = "mediatek,mt6351";
> +			interrupt-controller;

the child node can't be added until MT6351 support is added to
dt-binding

> +		};
> +	};
> +
>  	uart0: serial@11002000 {
>  		compatible = "mediatek,mt6797-uart",
>  			     "mediatek,mt6577-uart";

[PATCH V4 8/8] arm64: dts: mt6797: add pwrap support for mt6797

[Sean Wang]

On Wed, 2018-05-02 at 17:21 +0800, argus.lin at mediatek.com wrote:
> From: Argus Lin <argus.lin@mediatek.com>
> 
> mt6797 is a highly integrated SoCs, and it uses
> mt6351 as Power Management IC.
> We need to add pwrap device to communicate with
> mt6351 by SPI.
> The base address of pwrap is 0x1000d000, and IRQ
> number is 178. It also using fixed 26Mhz clock
> as SPI CLK.
> 
> Signed-off-by: Argus Lin <argus.lin@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt6797.dtsi | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt6797.dtsi b/arch/arm64/boot/dts/mediatek/mt6797.dtsi
> index 4beaa71107d7..485546efc9bf 100644
> --- a/arch/arm64/boot/dts/mediatek/mt6797.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt6797.dtsi
> @@ -161,6 +161,20 @@
>  		      <0 0x10220690 0 0x10>;
>  	};
>  
> +	pwrap: pwrap at 1000d000 {
> +		compatible = "mediatek,mt6797-pwrap";
> +		reg = <0 0x1000d000 0 0x1000>;
> +		reg-names = "pwrap";
> +		interrupts = <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&clk26m>, <&clk26m>;
> +		clock-names = "spi", "wrap";
> +
> +		pmic: mt6351 {
> +			compatible = "mediatek,mt6351";
> +			interrupt-controller;

the child node can't be added until MT6351 support is added to
dt-binding

> +		};
> +	};
> +
>  	uart0: serial at 11002000 {
>  		compatible = "mediatek,mt6797-uart",
>  			     "mediatek,mt6577-uart";

Re: [PATCH V4 7/8] soc: mediatek: pwrap: add mt6351 for mt6797 SoCs

[Sean Wang]

Hi, Argus

On Wed, 2018-05-02 at 17:21 +0800, argus.lin@mediatek.com wrote:
> From: Argus Lin <argus.lin@mediatek.com>
> 
> mt6351 is a new power management IC and it is
> used for mt6797 SoCs. We need to add mt6351_regs for
> pmic register mapping and pmic_mt6351 for
> register accessing by regmap.
> 

suggest line wrapping closely at 75 columns

> Signed-off-by: Argus Lin <argus.lin@mediatek.com>
> ---
>  drivers/soc/mediatek/mtk-pmic-wrap.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index 285bfa76249f..26076900eee0 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -152,6 +152,21 @@ static const u32 mt6397_regs[] = {
>  	[PWRAP_DEW_CIPHER_SWRST] =	0xbc24,
>  };
>  
> +static const u32 mt6351_regs[] = {
> +	[PWRAP_DEW_DIO_EN] =		0x02F2,
> +	[PWRAP_DEW_READ_TEST] =		0x02F4,
> +	[PWRAP_DEW_WRITE_TEST] =	0x02F6,
> +	[PWRAP_DEW_CRC_EN] =		0x02FA,
> +	[PWRAP_DEW_CRC_VAL] =		0x02FC,
> +	[PWRAP_DEW_CIPHER_KEY_SEL] =	0x0300,
> +	[PWRAP_DEW_CIPHER_IV_SEL] =	0x0302,
> +	[PWRAP_DEW_CIPHER_EN] =		0x0304,
> +	[PWRAP_DEW_CIPHER_RDY] =	0x0306,
> +	[PWRAP_DEW_CIPHER_MODE] =	0x0308,
> +	[PWRAP_DEW_CIPHER_SWRST] =	0x030A,
> +	[PWRAP_DEW_RDDMY_NO] =		0x030C,
> +};
> +

trim the unused registers if any

>  enum pwrap_regs {
>  	PWRAP_MUX_SEL,
>  	PWRAP_WRAP_EN,
> @@ -684,6 +699,7 @@ static int mt8135_regs[] = {
>  
>  enum pmic_type {
>  	PMIC_MT6323,
> +	PMIC_MT6351,
>  	PMIC_MT6380,
>  	PMIC_MT6397,
>  };
> @@ -1150,6 +1166,7 @@ static int pwrap_init_cipher(struct pmic_wrapper *wrp)
>  			    0x1);
>  		break;
>  	case PMIC_MT6323:
> +	case PMIC_MT6351:
>  		pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_CIPHER_EN],
>  			    0x1);
>  		break;
> @@ -1435,6 +1452,15 @@ static const struct pwrap_slv_type pmic_mt6397 = {
>  	.pwrap_write = pwrap_write16,
>  };
>  
> +static const struct pwrap_slv_type pmic_mt6351 = {
> +	.dew_regs = mt6351_regs,
> +	.type = PMIC_MT6351,
> +	.regmap = &pwrap_regmap_config16,
> +	.caps = 0,

the caps should be

PWRAP_SLV_CAP_SPI | PWRAP_SLV_CAP_DUALIO |
                 PWRAP_SLV_CAP_SECURITY,
 
otherwise, the registers you defined here cannot be accessed by its
function.


> +	.pwrap_read = pwrap_read16,
> +	.pwrap_write = pwrap_write16,
> +};
> +
>  static const struct of_device_id of_slave_match_tbl[] = {
>  	{
>  		.compatible = "mediatek,mt6323",
> @@ -1449,6 +1475,9 @@ static const struct of_device_id of_slave_match_tbl[] = {
>  		.compatible = "mediatek,mt6397",
>  		.data = &pmic_mt6397,
>  	}, {
> +		.compatible = "mediatek,mt6351",
> +		.data = &pmic_mt6351,
> +	}, {

need to be sorted din alphabetical order

>  		/* sentinel */
>  	}
>  };

Re: [PATCH V4 7/8] soc: mediatek: pwrap: add mt6351 for mt6797 SoCs

[Sean Wang]

Hi, Argus

On Wed, 2018-05-02 at 17:21 +0800, argus.lin@mediatek.com wrote:
> From: Argus Lin <argus.lin@mediatek.com>
> 
> mt6351 is a new power management IC and it is
> used for mt6797 SoCs. We need to add mt6351_regs for
> pmic register mapping and pmic_mt6351 for
> register accessing by regmap.
> 

suggest line wrapping closely at 75 columns

> Signed-off-by: Argus Lin <argus.lin@mediatek.com>
> ---
>  drivers/soc/mediatek/mtk-pmic-wrap.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index 285bfa76249f..26076900eee0 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -152,6 +152,21 @@ static const u32 mt6397_regs[] = {
>  	[PWRAP_DEW_CIPHER_SWRST] =	0xbc24,
>  };
>  
> +static const u32 mt6351_regs[] = {
> +	[PWRAP_DEW_DIO_EN] =		0x02F2,
> +	[PWRAP_DEW_READ_TEST] =		0x02F4,
> +	[PWRAP_DEW_WRITE_TEST] =	0x02F6,
> +	[PWRAP_DEW_CRC_EN] =		0x02FA,
> +	[PWRAP_DEW_CRC_VAL] =		0x02FC,
> +	[PWRAP_DEW_CIPHER_KEY_SEL] =	0x0300,
> +	[PWRAP_DEW_CIPHER_IV_SEL] =	0x0302,
> +	[PWRAP_DEW_CIPHER_EN] =		0x0304,
> +	[PWRAP_DEW_CIPHER_RDY] =	0x0306,
> +	[PWRAP_DEW_CIPHER_MODE] =	0x0308,
> +	[PWRAP_DEW_CIPHER_SWRST] =	0x030A,
> +	[PWRAP_DEW_RDDMY_NO] =		0x030C,
> +};
> +

trim the unused registers if any

>  enum pwrap_regs {
>  	PWRAP_MUX_SEL,
>  	PWRAP_WRAP_EN,
> @@ -684,6 +699,7 @@ static int mt8135_regs[] = {
>  
>  enum pmic_type {
>  	PMIC_MT6323,
> +	PMIC_MT6351,
>  	PMIC_MT6380,
>  	PMIC_MT6397,
>  };
> @@ -1150,6 +1166,7 @@ static int pwrap_init_cipher(struct pmic_wrapper *wrp)
>  			    0x1);
>  		break;
>  	case PMIC_MT6323:
> +	case PMIC_MT6351:
>  		pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_CIPHER_EN],
>  			    0x1);
>  		break;
> @@ -1435,6 +1452,15 @@ static const struct pwrap_slv_type pmic_mt6397 = {
>  	.pwrap_write = pwrap_write16,
>  };
>  
> +static const struct pwrap_slv_type pmic_mt6351 = {
> +	.dew_regs = mt6351_regs,
> +	.type = PMIC_MT6351,
> +	.regmap = &pwrap_regmap_config16,
> +	.caps = 0,

the caps should be

PWRAP_SLV_CAP_SPI | PWRAP_SLV_CAP_DUALIO |
                 PWRAP_SLV_CAP_SECURITY,
 
otherwise, the registers you defined here cannot be accessed by its
function.


> +	.pwrap_read = pwrap_read16,
> +	.pwrap_write = pwrap_write16,
> +};
> +
>  static const struct of_device_id of_slave_match_tbl[] = {
>  	{
>  		.compatible = "mediatek,mt6323",
> @@ -1449,6 +1475,9 @@ static const struct of_device_id of_slave_match_tbl[] = {
>  		.compatible = "mediatek,mt6397",
>  		.data = &pmic_mt6397,
>  	}, {
> +		.compatible = "mediatek,mt6351",
> +		.data = &pmic_mt6351,
> +	}, {

need to be sorted din alphabetical order

>  		/* sentinel */
>  	}
>  };

[PATCH V4 7/8] soc: mediatek: pwrap: add mt6351 for mt6797 SoCs

[Sean Wang]

Hi, Argus

On Wed, 2018-05-02 at 17:21 +0800, argus.lin at mediatek.com wrote:
> From: Argus Lin <argus.lin@mediatek.com>
> 
> mt6351 is a new power management IC and it is
> used for mt6797 SoCs. We need to add mt6351_regs for
> pmic register mapping and pmic_mt6351 for
> register accessing by regmap.
> 

suggest line wrapping closely at 75 columns

> Signed-off-by: Argus Lin <argus.lin@mediatek.com>
> ---
>  drivers/soc/mediatek/mtk-pmic-wrap.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index 285bfa76249f..26076900eee0 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -152,6 +152,21 @@ static const u32 mt6397_regs[] = {
>  	[PWRAP_DEW_CIPHER_SWRST] =	0xbc24,
>  };
>  
> +static const u32 mt6351_regs[] = {
> +	[PWRAP_DEW_DIO_EN] =		0x02F2,
> +	[PWRAP_DEW_READ_TEST] =		0x02F4,
> +	[PWRAP_DEW_WRITE_TEST] =	0x02F6,
> +	[PWRAP_DEW_CRC_EN] =		0x02FA,
> +	[PWRAP_DEW_CRC_VAL] =		0x02FC,
> +	[PWRAP_DEW_CIPHER_KEY_SEL] =	0x0300,
> +	[PWRAP_DEW_CIPHER_IV_SEL] =	0x0302,
> +	[PWRAP_DEW_CIPHER_EN] =		0x0304,
> +	[PWRAP_DEW_CIPHER_RDY] =	0x0306,
> +	[PWRAP_DEW_CIPHER_MODE] =	0x0308,
> +	[PWRAP_DEW_CIPHER_SWRST] =	0x030A,
> +	[PWRAP_DEW_RDDMY_NO] =		0x030C,
> +};
> +

trim the unused registers if any

>  enum pwrap_regs {
>  	PWRAP_MUX_SEL,
>  	PWRAP_WRAP_EN,
> @@ -684,6 +699,7 @@ static int mt8135_regs[] = {
>  
>  enum pmic_type {
>  	PMIC_MT6323,
> +	PMIC_MT6351,
>  	PMIC_MT6380,
>  	PMIC_MT6397,
>  };
> @@ -1150,6 +1166,7 @@ static int pwrap_init_cipher(struct pmic_wrapper *wrp)
>  			    0x1);
>  		break;
>  	case PMIC_MT6323:
> +	case PMIC_MT6351:
>  		pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_CIPHER_EN],
>  			    0x1);
>  		break;
> @@ -1435,6 +1452,15 @@ static const struct pwrap_slv_type pmic_mt6397 = {
>  	.pwrap_write = pwrap_write16,
>  };
>  
> +static const struct pwrap_slv_type pmic_mt6351 = {
> +	.dew_regs = mt6351_regs,
> +	.type = PMIC_MT6351,
> +	.regmap = &pwrap_regmap_config16,
> +	.caps = 0,

the caps should be

PWRAP_SLV_CAP_SPI | PWRAP_SLV_CAP_DUALIO |
                 PWRAP_SLV_CAP_SECURITY,
 
otherwise, the registers you defined here cannot be accessed by its
function.


> +	.pwrap_read = pwrap_read16,
> +	.pwrap_write = pwrap_write16,
> +};
> +
>  static const struct of_device_id of_slave_match_tbl[] = {
>  	{
>  		.compatible = "mediatek,mt6323",
> @@ -1449,6 +1475,9 @@ static const struct of_device_id of_slave_match_tbl[] = {
>  		.compatible = "mediatek,mt6397",
>  		.data = &pmic_mt6397,
>  	}, {
> +		.compatible = "mediatek,mt6351",
> +		.data = &pmic_mt6351,
> +	}, {

need to be sorted din alphabetical order

>  		/* sentinel */
>  	}
>  };

Re: [PATCH V4 5/8] soc: mediatek: pwrap: add pwrap for mt6797 SoCs

[Sean Wang]

Hi, Argus

On Wed, 2018-05-02 at 17:21 +0800, argus.lin@mediatek.com wrote:
> From: Argus Lin <argus.lin@mediatek.com>
> 
> mt6797 is a highly integrated SoCs, it uses mt6351 for power
> management. We need to add pwrap support to access mt6351.
> Pwrap of mt6797 support new feature include starvation and channel
> request exception interrupt, dynamic starvation priority
> adjustment mechanism.

suggest line wrapping closely at 75 columns

> 
> Signed-off-by: Argus Lin <argus.lin@mediatek.com>
> ---
>  drivers/soc/mediatek/mtk-pmic-wrap.c | 110 ++++++++++++++++++++++++++++++++---
>  1 file changed, 102 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index a6366f147b79..0d4a2dae6912 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -284,6 +284,12 @@ enum pwrap_regs {
>  	PWRAP_DVFS_WDATA7,
>  	PWRAP_SPMINF_STA,
>  	PWRAP_CIPHER_EN,
> +
> +	/* MT6797 series regs */
> +	PWRAP_INT1_EN,
> +	PWRAP_INT1_FLG_RAW,
> +	PWRAP_INT1_FLG,
> +	PWRAP_INT1_CLR,
>  };
>  
>  static int mt2701_regs[] = {
> @@ -372,6 +378,43 @@ static int mt2701_regs[] = {
>  	[PWRAP_ADC_RDATA_ADDR2] =	0x154,
>  };
>  
> +static int mt6797_regs[] = {
> +	[PWRAP_MUX_SEL] =		0x0,
> +	[PWRAP_WRAP_EN] =		0x4,
> +	[PWRAP_DIO_EN] =		0x8,
> +	[PWRAP_SIDLY] =			0xC,
> +	[PWRAP_RDDMY] =			0x10,
> +	[PWRAP_CSHEXT_WRITE] =		0x18,
> +	[PWRAP_CSHEXT_READ] =		0x1C,
> +	[PWRAP_CSLEXT_START] =		0x20,
> +	[PWRAP_CSLEXT_END] =		0x24,
> +	[PWRAP_STAUPD_PRD] =		0x28,
> +	[PWRAP_HARB_HPRIO] =		0x50,
> +	[PWRAP_HIPRIO_ARB_EN] =		0x54,
> +	[PWRAP_MAN_EN] =		0x60,
> +	[PWRAP_MAN_CMD] =		0x64,
> +	[PWRAP_WACS0_EN] =		0x70,
> +	[PWRAP_WACS1_EN] =		0x84,
> +	[PWRAP_WACS2_EN] =		0x98,
> +	[PWRAP_INIT_DONE2] =		0x9C,
> +	[PWRAP_WACS2_CMD] =		0xA0,
> +	[PWRAP_WACS2_RDATA] =		0xA4,
> +	[PWRAP_WACS2_VLDCLR] =		0xA8,
> +	[PWRAP_INT_EN] =		0xC0,
> +	[PWRAP_INT_FLG_RAW] =		0xC4,
> +	[PWRAP_INT_FLG] =		0xC8,
> +	[PWRAP_INT_CLR] =		0xCC,
> +	[PWRAP_INT1_EN] =		0xD0,
> +	[PWRAP_INT1_FLG_RAW] =		0xD4,
> +	[PWRAP_INT1_FLG] =		0xD8,
> +	[PWRAP_INT1_CLR] =		0xDC,
> +	[PWRAP_TIMER_EN] =		0xF4,
> +	[PWRAP_WDT_UNIT] =		0xFC,
> +	[PWRAP_WDT_SRC_EN] =		0x100,
> +	[PWRAP_DCM_EN] =		0x1CC,
> +	[PWRAP_DCM_DBC_PRD] =		0x1D4,
> +};
> +

trim unused registers if any

>  static int mt7622_regs[] = {
>  	[PWRAP_MUX_SEL] =		0x0,
>  	[PWRAP_WRAP_EN] =		0x4,
> @@ -647,6 +690,7 @@ enum pmic_type {
>  
>  enum pwrap_type {
>  	PWRAP_MT2701,
> +	PWRAP_MT6797,
>  	PWRAP_MT7622,
>  	PWRAP_MT8135,
>  	PWRAP_MT8173,
> @@ -1006,6 +1050,12 @@ static void pwrap_init_chip_select_ext(struct pmic_wrapper *wrp, u8 hext_write,
>  static int pwrap_common_init_reg_clock(struct pmic_wrapper *wrp)
>  {
>  	switch (wrp->master->type) {
> +	case PWRAP_MT6797:
> +		pwrap_writel(wrp, 0x8, PWRAP_RDDMY);
> +		pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_RDDMY_NO],
> +			    0x8);
> +		pwrap_init_chip_select_ext(wrp, 0x88, 0x55, 3, 0);
> +		break;

the setup for timing is much similar to mt2701 + mt6323

so we can merge the both logic into one, and then hope to eliminate specific pwrap_mt2701_init_reg_clock totally

>  	case PWRAP_MT8173:
>  		pwrap_init_chip_select_ext(wrp, 0, 4, 2, 2);
>  		break;
> @@ -1076,11 +1126,14 @@ static int pwrap_init_cipher(struct pmic_wrapper *wrp)
>  		break;
>  	case PWRAP_MT2701:
>  	case PWRAP_MT8173:
> +	case PWRAP_MT6797:

need to be listed in alphabetical order

>  		pwrap_writel(wrp, 1, PWRAP_CIPHER_EN);
>  		break;
>  	case PWRAP_MT7622:
>  		pwrap_writel(wrp, 0, PWRAP_CIPHER_EN);
>  		break;
> +	default:
> +		break;
>  	}
>  
>  	/* Config cipher mode @PMIC */
> @@ -1325,6 +1378,15 @@ static irqreturn_t pwrap_interrupt(int irqno, void *dev_id)
>  
>  	pwrap_writel(wrp, 0xffffffff, PWRAP_INT_CLR);
>  
> +	/* If we support INT1 interrupt, we also need to clear it */
> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_INT1_EN)) {
> +		rdata = pwrap_readl(wrp, PWRAP_INT1_FLG);
> +
> +		dev_err(wrp->dev, "unexpected interrupt int1=0x%x\n", rdata);
> +
> +		pwrap_writel(wrp, rdata, PWRAP_INT1_CLR);
> +	}
> +

it seems no required to add PWRAP_CAP_INT1_EN:

the CAP is not to take any action for any INTs raised, instead, it can be disabled at initial.

>  	return IRQ_HANDLED;
>  }
>  
> @@ -1446,6 +1508,19 @@ static const struct pmic_wrapper_type pwrap_mt8173 = {
>  	.init_soc_specific = pwrap_mt8173_init_soc_specific,
>  };
>  
> +static const struct pmic_wrapper_type pwrap_mt6797 = {
> +	.regs = mt6797_regs,
> +	.type = PWRAP_MT6797,
> +	.arb_en_all = 0x01fff,
> +	.int_en_all = 0xffffffc6,
> +	.int1_en_all = 0x0001ffff,
> +	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
> +	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
> +	.caps = PWRAP_CAP_DCM | PWRAP_CAP_INT1_EN,
> +	.init_reg_clock = pwrap_common_init_reg_clock,
> +	.init_soc_specific = NULL,
> +};
> +
>  static const struct of_device_id of_pwrap_match_tbl[] = {
>  	{
>  		.compatible = "mediatek,mt2701-pwrap",
> @@ -1460,6 +1535,9 @@ static const struct of_device_id of_pwrap_match_tbl[] = {
>  		.compatible = "mediatek,mt8173-pwrap",
>  		.data = &pwrap_mt8173,
>  	}, {
> +		.compatible = "mediatek,mt6797-pwrap",
> +		.data = &pwrap_mt6797,

each entry needs to be sorted in alphabetical order

> +	}, {
>  		/* sentinel */
>  	}
>  };
> @@ -1503,11 +1581,13 @@ static int pwrap_probe(struct platform_device *pdev)
>  	if (IS_ERR(wrp->base))
>  		return PTR_ERR(wrp->base);
>  
> -	wrp->rstc = devm_reset_control_get(wrp->dev, "pwrap");
> -	if (IS_ERR(wrp->rstc)) {
> -		ret = PTR_ERR(wrp->rstc);
> -		dev_dbg(wrp->dev, "cannot get pwrap reset: %d\n", ret);
> -		return ret;
> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_RESET)) {
> +		wrp->rstc = devm_reset_control_get(wrp->dev, "pwrap");

there should be a reset bit present for pwrap on infrasys.

the specific condition can be dropped when the reset cell is exported from infrasys and then the device has a reference to it.

> +		if (IS_ERR(wrp->rstc)) {
> +			ret = PTR_ERR(wrp->rstc);
> +			dev_dbg(wrp->dev, "cannot get pwrap reset: %d\n", ret);
> +			return ret;
> +		}
>  	}
>  
>  	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_BRIDGE)) {
> @@ -1549,9 +1629,17 @@ static int pwrap_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err_out1;
>  
> -	/* Enable internal dynamic clock */
> -	pwrap_writel(wrp, 1, PWRAP_DCM_EN);
> -	pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
> +	/*
> +	 * add dcm capability check
> +	 */
> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_DCM)) {

the specific condition can be dropped since so far all devices the driver can support are owning PWRAP_CAP_DCM

> +		if (wrp->master->type == PWRAP_MT6797)
> +			pwrap_writel(wrp, 3, PWRAP_DCM_EN);

the setup for MT6797 can be moved into .init_soc_specific callback ?

> +		else
> +			pwrap_writel(wrp, 1, PWRAP_DCM_EN);
> +
> +		pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
> +	}
>  
>  	/*
>  	 * The PMIC could already be initialized by the bootloader.
> @@ -1580,6 +1668,12 @@ static int pwrap_probe(struct platform_device *pdev)
>  	pwrap_writel(wrp, wrp->master->wdt_src, PWRAP_WDT_SRC_EN);
>  	pwrap_writel(wrp, 0x1, PWRAP_TIMER_EN);
>  	pwrap_writel(wrp, wrp->master->int_en_all, PWRAP_INT_EN);
> +	/*
> +	 * We add INT1 interrupt to handle starvation and request exception
> +	 * If we support it, we should enable them here.
> +	 */
> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_INT1_EN))
> +		pwrap_writel(wrp, wrp->master->int1_en_all, PWRAP_INT1_EN);
>  

if there is no explicitly enabling on INT1, then ISR handling for INT1 is also unnecessary

>  	irq = platform_get_irq(pdev, 0);
>  	ret = devm_request_irq(wrp->dev, irq, pwrap_interrupt,

Re: [PATCH V4 5/8] soc: mediatek: pwrap: add pwrap for mt6797 SoCs

[Sean Wang]

Hi, Argus

On Wed, 2018-05-02 at 17:21 +0800, argus.lin@mediatek.com wrote:
> From: Argus Lin <argus.lin@mediatek.com>
> 
> mt6797 is a highly integrated SoCs, it uses mt6351 for power
> management. We need to add pwrap support to access mt6351.
> Pwrap of mt6797 support new feature include starvation and channel
> request exception interrupt, dynamic starvation priority
> adjustment mechanism.

suggest line wrapping closely at 75 columns

> 
> Signed-off-by: Argus Lin <argus.lin@mediatek.com>
> ---
>  drivers/soc/mediatek/mtk-pmic-wrap.c | 110 ++++++++++++++++++++++++++++++++---
>  1 file changed, 102 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index a6366f147b79..0d4a2dae6912 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -284,6 +284,12 @@ enum pwrap_regs {
>  	PWRAP_DVFS_WDATA7,
>  	PWRAP_SPMINF_STA,
>  	PWRAP_CIPHER_EN,
> +
> +	/* MT6797 series regs */
> +	PWRAP_INT1_EN,
> +	PWRAP_INT1_FLG_RAW,
> +	PWRAP_INT1_FLG,
> +	PWRAP_INT1_CLR,
>  };
>  
>  static int mt2701_regs[] = {
> @@ -372,6 +378,43 @@ static int mt2701_regs[] = {
>  	[PWRAP_ADC_RDATA_ADDR2] =	0x154,
>  };
>  
> +static int mt6797_regs[] = {
> +	[PWRAP_MUX_SEL] =		0x0,
> +	[PWRAP_WRAP_EN] =		0x4,
> +	[PWRAP_DIO_EN] =		0x8,
> +	[PWRAP_SIDLY] =			0xC,
> +	[PWRAP_RDDMY] =			0x10,
> +	[PWRAP_CSHEXT_WRITE] =		0x18,
> +	[PWRAP_CSHEXT_READ] =		0x1C,
> +	[PWRAP_CSLEXT_START] =		0x20,
> +	[PWRAP_CSLEXT_END] =		0x24,
> +	[PWRAP_STAUPD_PRD] =		0x28,
> +	[PWRAP_HARB_HPRIO] =		0x50,
> +	[PWRAP_HIPRIO_ARB_EN] =		0x54,
> +	[PWRAP_MAN_EN] =		0x60,
> +	[PWRAP_MAN_CMD] =		0x64,
> +	[PWRAP_WACS0_EN] =		0x70,
> +	[PWRAP_WACS1_EN] =		0x84,
> +	[PWRAP_WACS2_EN] =		0x98,
> +	[PWRAP_INIT_DONE2] =		0x9C,
> +	[PWRAP_WACS2_CMD] =		0xA0,
> +	[PWRAP_WACS2_RDATA] =		0xA4,
> +	[PWRAP_WACS2_VLDCLR] =		0xA8,
> +	[PWRAP_INT_EN] =		0xC0,
> +	[PWRAP_INT_FLG_RAW] =		0xC4,
> +	[PWRAP_INT_FLG] =		0xC8,
> +	[PWRAP_INT_CLR] =		0xCC,
> +	[PWRAP_INT1_EN] =		0xD0,
> +	[PWRAP_INT1_FLG_RAW] =		0xD4,
> +	[PWRAP_INT1_FLG] =		0xD8,
> +	[PWRAP_INT1_CLR] =		0xDC,
> +	[PWRAP_TIMER_EN] =		0xF4,
> +	[PWRAP_WDT_UNIT] =		0xFC,
> +	[PWRAP_WDT_SRC_EN] =		0x100,
> +	[PWRAP_DCM_EN] =		0x1CC,
> +	[PWRAP_DCM_DBC_PRD] =		0x1D4,
> +};
> +

trim unused registers if any

>  static int mt7622_regs[] = {
>  	[PWRAP_MUX_SEL] =		0x0,
>  	[PWRAP_WRAP_EN] =		0x4,
> @@ -647,6 +690,7 @@ enum pmic_type {
>  
>  enum pwrap_type {
>  	PWRAP_MT2701,
> +	PWRAP_MT6797,
>  	PWRAP_MT7622,
>  	PWRAP_MT8135,
>  	PWRAP_MT8173,
> @@ -1006,6 +1050,12 @@ static void pwrap_init_chip_select_ext(struct pmic_wrapper *wrp, u8 hext_write,
>  static int pwrap_common_init_reg_clock(struct pmic_wrapper *wrp)
>  {
>  	switch (wrp->master->type) {
> +	case PWRAP_MT6797:
> +		pwrap_writel(wrp, 0x8, PWRAP_RDDMY);
> +		pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_RDDMY_NO],
> +			    0x8);
> +		pwrap_init_chip_select_ext(wrp, 0x88, 0x55, 3, 0);
> +		break;

the setup for timing is much similar to mt2701 + mt6323

so we can merge the both logic into one, and then hope to eliminate specific pwrap_mt2701_init_reg_clock totally

>  	case PWRAP_MT8173:
>  		pwrap_init_chip_select_ext(wrp, 0, 4, 2, 2);
>  		break;
> @@ -1076,11 +1126,14 @@ static int pwrap_init_cipher(struct pmic_wrapper *wrp)
>  		break;
>  	case PWRAP_MT2701:
>  	case PWRAP_MT8173:
> +	case PWRAP_MT6797:

need to be listed in alphabetical order

>  		pwrap_writel(wrp, 1, PWRAP_CIPHER_EN);
>  		break;
>  	case PWRAP_MT7622:
>  		pwrap_writel(wrp, 0, PWRAP_CIPHER_EN);
>  		break;
> +	default:
> +		break;
>  	}
>  
>  	/* Config cipher mode @PMIC */
> @@ -1325,6 +1378,15 @@ static irqreturn_t pwrap_interrupt(int irqno, void *dev_id)
>  
>  	pwrap_writel(wrp, 0xffffffff, PWRAP_INT_CLR);
>  
> +	/* If we support INT1 interrupt, we also need to clear it */
> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_INT1_EN)) {
> +		rdata = pwrap_readl(wrp, PWRAP_INT1_FLG);
> +
> +		dev_err(wrp->dev, "unexpected interrupt int1=0x%x\n", rdata);
> +
> +		pwrap_writel(wrp, rdata, PWRAP_INT1_CLR);
> +	}
> +

it seems no required to add PWRAP_CAP_INT1_EN:

the CAP is not to take any action for any INTs raised, instead, it can be disabled at initial.

>  	return IRQ_HANDLED;
>  }
>  
> @@ -1446,6 +1508,19 @@ static const struct pmic_wrapper_type pwrap_mt8173 = {
>  	.init_soc_specific = pwrap_mt8173_init_soc_specific,
>  };
>  
> +static const struct pmic_wrapper_type pwrap_mt6797 = {
> +	.regs = mt6797_regs,
> +	.type = PWRAP_MT6797,
> +	.arb_en_all = 0x01fff,
> +	.int_en_all = 0xffffffc6,
> +	.int1_en_all = 0x0001ffff,
> +	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
> +	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
> +	.caps = PWRAP_CAP_DCM | PWRAP_CAP_INT1_EN,
> +	.init_reg_clock = pwrap_common_init_reg_clock,
> +	.init_soc_specific = NULL,
> +};
> +
>  static const struct of_device_id of_pwrap_match_tbl[] = {
>  	{
>  		.compatible = "mediatek,mt2701-pwrap",
> @@ -1460,6 +1535,9 @@ static const struct of_device_id of_pwrap_match_tbl[] = {
>  		.compatible = "mediatek,mt8173-pwrap",
>  		.data = &pwrap_mt8173,
>  	}, {
> +		.compatible = "mediatek,mt6797-pwrap",
> +		.data = &pwrap_mt6797,

each entry needs to be sorted in alphabetical order

> +	}, {
>  		/* sentinel */
>  	}
>  };
> @@ -1503,11 +1581,13 @@ static int pwrap_probe(struct platform_device *pdev)
>  	if (IS_ERR(wrp->base))
>  		return PTR_ERR(wrp->base);
>  
> -	wrp->rstc = devm_reset_control_get(wrp->dev, "pwrap");
> -	if (IS_ERR(wrp->rstc)) {
> -		ret = PTR_ERR(wrp->rstc);
> -		dev_dbg(wrp->dev, "cannot get pwrap reset: %d\n", ret);
> -		return ret;
> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_RESET)) {
> +		wrp->rstc = devm_reset_control_get(wrp->dev, "pwrap");

there should be a reset bit present for pwrap on infrasys.

the specific condition can be dropped when the reset cell is exported from infrasys and then the device has a reference to it.

> +		if (IS_ERR(wrp->rstc)) {
> +			ret = PTR_ERR(wrp->rstc);
> +			dev_dbg(wrp->dev, "cannot get pwrap reset: %d\n", ret);
> +			return ret;
> +		}
>  	}
>  
>  	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_BRIDGE)) {
> @@ -1549,9 +1629,17 @@ static int pwrap_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err_out1;
>  
> -	/* Enable internal dynamic clock */
> -	pwrap_writel(wrp, 1, PWRAP_DCM_EN);
> -	pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
> +	/*
> +	 * add dcm capability check
> +	 */
> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_DCM)) {

the specific condition can be dropped since so far all devices the driver can support are owning PWRAP_CAP_DCM

> +		if (wrp->master->type == PWRAP_MT6797)
> +			pwrap_writel(wrp, 3, PWRAP_DCM_EN);

the setup for MT6797 can be moved into .init_soc_specific callback ?

> +		else
> +			pwrap_writel(wrp, 1, PWRAP_DCM_EN);
> +
> +		pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
> +	}
>  
>  	/*
>  	 * The PMIC could already be initialized by the bootloader.
> @@ -1580,6 +1668,12 @@ static int pwrap_probe(struct platform_device *pdev)
>  	pwrap_writel(wrp, wrp->master->wdt_src, PWRAP_WDT_SRC_EN);
>  	pwrap_writel(wrp, 0x1, PWRAP_TIMER_EN);
>  	pwrap_writel(wrp, wrp->master->int_en_all, PWRAP_INT_EN);
> +	/*
> +	 * We add INT1 interrupt to handle starvation and request exception
> +	 * If we support it, we should enable them here.
> +	 */
> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_INT1_EN))
> +		pwrap_writel(wrp, wrp->master->int1_en_all, PWRAP_INT1_EN);
>  

if there is no explicitly enabling on INT1, then ISR handling for INT1 is also unnecessary

>  	irq = platform_get_irq(pdev, 0);
>  	ret = devm_request_irq(wrp->dev, irq, pwrap_interrupt,

[PATCH V4 5/8] soc: mediatek: pwrap: add pwrap for mt6797 SoCs

[Sean Wang]

Hi, Argus

On Wed, 2018-05-02 at 17:21 +0800, argus.lin at mediatek.com wrote:
> From: Argus Lin <argus.lin@mediatek.com>
> 
> mt6797 is a highly integrated SoCs, it uses mt6351 for power
> management. We need to add pwrap support to access mt6351.
> Pwrap of mt6797 support new feature include starvation and channel
> request exception interrupt, dynamic starvation priority
> adjustment mechanism.

suggest line wrapping closely at 75 columns

> 
> Signed-off-by: Argus Lin <argus.lin@mediatek.com>
> ---
>  drivers/soc/mediatek/mtk-pmic-wrap.c | 110 ++++++++++++++++++++++++++++++++---
>  1 file changed, 102 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index a6366f147b79..0d4a2dae6912 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -284,6 +284,12 @@ enum pwrap_regs {
>  	PWRAP_DVFS_WDATA7,
>  	PWRAP_SPMINF_STA,
>  	PWRAP_CIPHER_EN,
> +
> +	/* MT6797 series regs */
> +	PWRAP_INT1_EN,
> +	PWRAP_INT1_FLG_RAW,
> +	PWRAP_INT1_FLG,
> +	PWRAP_INT1_CLR,
>  };
>  
>  static int mt2701_regs[] = {
> @@ -372,6 +378,43 @@ static int mt2701_regs[] = {
>  	[PWRAP_ADC_RDATA_ADDR2] =	0x154,
>  };
>  
> +static int mt6797_regs[] = {
> +	[PWRAP_MUX_SEL] =		0x0,
> +	[PWRAP_WRAP_EN] =		0x4,
> +	[PWRAP_DIO_EN] =		0x8,
> +	[PWRAP_SIDLY] =			0xC,
> +	[PWRAP_RDDMY] =			0x10,
> +	[PWRAP_CSHEXT_WRITE] =		0x18,
> +	[PWRAP_CSHEXT_READ] =		0x1C,
> +	[PWRAP_CSLEXT_START] =		0x20,
> +	[PWRAP_CSLEXT_END] =		0x24,
> +	[PWRAP_STAUPD_PRD] =		0x28,
> +	[PWRAP_HARB_HPRIO] =		0x50,
> +	[PWRAP_HIPRIO_ARB_EN] =		0x54,
> +	[PWRAP_MAN_EN] =		0x60,
> +	[PWRAP_MAN_CMD] =		0x64,
> +	[PWRAP_WACS0_EN] =		0x70,
> +	[PWRAP_WACS1_EN] =		0x84,
> +	[PWRAP_WACS2_EN] =		0x98,
> +	[PWRAP_INIT_DONE2] =		0x9C,
> +	[PWRAP_WACS2_CMD] =		0xA0,
> +	[PWRAP_WACS2_RDATA] =		0xA4,
> +	[PWRAP_WACS2_VLDCLR] =		0xA8,
> +	[PWRAP_INT_EN] =		0xC0,
> +	[PWRAP_INT_FLG_RAW] =		0xC4,
> +	[PWRAP_INT_FLG] =		0xC8,
> +	[PWRAP_INT_CLR] =		0xCC,
> +	[PWRAP_INT1_EN] =		0xD0,
> +	[PWRAP_INT1_FLG_RAW] =		0xD4,
> +	[PWRAP_INT1_FLG] =		0xD8,
> +	[PWRAP_INT1_CLR] =		0xDC,
> +	[PWRAP_TIMER_EN] =		0xF4,
> +	[PWRAP_WDT_UNIT] =		0xFC,
> +	[PWRAP_WDT_SRC_EN] =		0x100,
> +	[PWRAP_DCM_EN] =		0x1CC,
> +	[PWRAP_DCM_DBC_PRD] =		0x1D4,
> +};
> +

trim unused registers if any

>  static int mt7622_regs[] = {
>  	[PWRAP_MUX_SEL] =		0x0,
>  	[PWRAP_WRAP_EN] =		0x4,
> @@ -647,6 +690,7 @@ enum pmic_type {
>  
>  enum pwrap_type {
>  	PWRAP_MT2701,
> +	PWRAP_MT6797,
>  	PWRAP_MT7622,
>  	PWRAP_MT8135,
>  	PWRAP_MT8173,
> @@ -1006,6 +1050,12 @@ static void pwrap_init_chip_select_ext(struct pmic_wrapper *wrp, u8 hext_write,
>  static int pwrap_common_init_reg_clock(struct pmic_wrapper *wrp)
>  {
>  	switch (wrp->master->type) {
> +	case PWRAP_MT6797:
> +		pwrap_writel(wrp, 0x8, PWRAP_RDDMY);
> +		pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_RDDMY_NO],
> +			    0x8);
> +		pwrap_init_chip_select_ext(wrp, 0x88, 0x55, 3, 0);
> +		break;

the setup for timing is much similar to mt2701 + mt6323

so we can merge the both logic into one, and then hope to eliminate specific pwrap_mt2701_init_reg_clock totally

>  	case PWRAP_MT8173:
>  		pwrap_init_chip_select_ext(wrp, 0, 4, 2, 2);
>  		break;
> @@ -1076,11 +1126,14 @@ static int pwrap_init_cipher(struct pmic_wrapper *wrp)
>  		break;
>  	case PWRAP_MT2701:
>  	case PWRAP_MT8173:
> +	case PWRAP_MT6797:

need to be listed in alphabetical order

>  		pwrap_writel(wrp, 1, PWRAP_CIPHER_EN);
>  		break;
>  	case PWRAP_MT7622:
>  		pwrap_writel(wrp, 0, PWRAP_CIPHER_EN);
>  		break;
> +	default:
> +		break;
>  	}
>  
>  	/* Config cipher mode @PMIC */
> @@ -1325,6 +1378,15 @@ static irqreturn_t pwrap_interrupt(int irqno, void *dev_id)
>  
>  	pwrap_writel(wrp, 0xffffffff, PWRAP_INT_CLR);
>  
> +	/* If we support INT1 interrupt, we also need to clear it */
> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_INT1_EN)) {
> +		rdata = pwrap_readl(wrp, PWRAP_INT1_FLG);
> +
> +		dev_err(wrp->dev, "unexpected interrupt int1=0x%x\n", rdata);
> +
> +		pwrap_writel(wrp, rdata, PWRAP_INT1_CLR);
> +	}
> +

it seems no required to add PWRAP_CAP_INT1_EN:

the CAP is not to take any action for any INTs raised, instead, it can be disabled at initial.

>  	return IRQ_HANDLED;
>  }
>  
> @@ -1446,6 +1508,19 @@ static const struct pmic_wrapper_type pwrap_mt8173 = {
>  	.init_soc_specific = pwrap_mt8173_init_soc_specific,
>  };
>  
> +static const struct pmic_wrapper_type pwrap_mt6797 = {
> +	.regs = mt6797_regs,
> +	.type = PWRAP_MT6797,
> +	.arb_en_all = 0x01fff,
> +	.int_en_all = 0xffffffc6,
> +	.int1_en_all = 0x0001ffff,
> +	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
> +	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
> +	.caps = PWRAP_CAP_DCM | PWRAP_CAP_INT1_EN,
> +	.init_reg_clock = pwrap_common_init_reg_clock,
> +	.init_soc_specific = NULL,
> +};
> +
>  static const struct of_device_id of_pwrap_match_tbl[] = {
>  	{
>  		.compatible = "mediatek,mt2701-pwrap",
> @@ -1460,6 +1535,9 @@ static const struct of_device_id of_pwrap_match_tbl[] = {
>  		.compatible = "mediatek,mt8173-pwrap",
>  		.data = &pwrap_mt8173,
>  	}, {
> +		.compatible = "mediatek,mt6797-pwrap",
> +		.data = &pwrap_mt6797,

each entry needs to be sorted in alphabetical order

> +	}, {
>  		/* sentinel */
>  	}
>  };
> @@ -1503,11 +1581,13 @@ static int pwrap_probe(struct platform_device *pdev)
>  	if (IS_ERR(wrp->base))
>  		return PTR_ERR(wrp->base);
>  
> -	wrp->rstc = devm_reset_control_get(wrp->dev, "pwrap");
> -	if (IS_ERR(wrp->rstc)) {
> -		ret = PTR_ERR(wrp->rstc);
> -		dev_dbg(wrp->dev, "cannot get pwrap reset: %d\n", ret);
> -		return ret;
> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_RESET)) {
> +		wrp->rstc = devm_reset_control_get(wrp->dev, "pwrap");

there should be a reset bit present for pwrap on infrasys.

the specific condition can be dropped when the reset cell is exported from infrasys and then the device has a reference to it.

> +		if (IS_ERR(wrp->rstc)) {
> +			ret = PTR_ERR(wrp->rstc);
> +			dev_dbg(wrp->dev, "cannot get pwrap reset: %d\n", ret);
> +			return ret;
> +		}
>  	}
>  
>  	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_BRIDGE)) {
> @@ -1549,9 +1629,17 @@ static int pwrap_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err_out1;
>  
> -	/* Enable internal dynamic clock */
> -	pwrap_writel(wrp, 1, PWRAP_DCM_EN);
> -	pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
> +	/*
> +	 * add dcm capability check
> +	 */
> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_DCM)) {

the specific condition can be dropped since so far all devices the driver can support are owning PWRAP_CAP_DCM

> +		if (wrp->master->type == PWRAP_MT6797)
> +			pwrap_writel(wrp, 3, PWRAP_DCM_EN);

the setup for MT6797 can be moved into .init_soc_specific callback ?

> +		else
> +			pwrap_writel(wrp, 1, PWRAP_DCM_EN);
> +
> +		pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
> +	}
>  
>  	/*
>  	 * The PMIC could already be initialized by the bootloader.
> @@ -1580,6 +1668,12 @@ static int pwrap_probe(struct platform_device *pdev)
>  	pwrap_writel(wrp, wrp->master->wdt_src, PWRAP_WDT_SRC_EN);
>  	pwrap_writel(wrp, 0x1, PWRAP_TIMER_EN);
>  	pwrap_writel(wrp, wrp->master->int_en_all, PWRAP_INT_EN);
> +	/*
> +	 * We add INT1 interrupt to handle starvation and request exception
> +	 * If we support it, we should enable them here.
> +	 */
> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_INT1_EN))
> +		pwrap_writel(wrp, wrp->master->int1_en_all, PWRAP_INT1_EN);
>  

if there is no explicitly enabling on INT1, then ISR handling for INT1 is also unnecessary

>  	irq = platform_get_irq(pdev, 0);
>  	ret = devm_request_irq(wrp->dev, irq, pwrap_interrupt,

Re: [PATCH V4 5/8] soc: mediatek: pwrap: add pwrap for mt6797 SoCs

[Argus Lin]

[-- Attachment #1.1: Type: text/html, Size: 16041 bytes --]

[-- Attachment #1.2: Type: text/plain, Size: 8985 bytes --]

On Thu, 2018-05-03 at 12:01 +0800, Sean Wang wrote:
> Hi, Argus
> 
> On Wed, 2018-05-02 at 17:21 +0800, argus.lin@mediatek.com wrote:
> > From: Argus Lin <argus.lin@mediatek.com>
> > 
> > mt6797 is a highly integrated SoCs, it uses mt6351 for power
> > management. We need to add pwrap support to access mt6351.
> > Pwrap of mt6797 support new feature include starvation and channel
> > request exception interrupt, dynamic starvation priority
> > adjustment mechanism.
> 
> suggest line wrapping closely at 75 columns
no problem
> 
> > 
> > Signed-off-by: Argus Lin <argus.lin@mediatek.com>
> > ---
> >  drivers/soc/mediatek/mtk-pmic-wrap.c | 110 ++++++++++++++++++++++++++++++++---
> >  1 file changed, 102 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> > index a6366f147b79..0d4a2dae6912 100644
> > --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> > +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> > @@ -284,6 +284,12 @@ enum pwrap_regs {
> >  	PWRAP_DVFS_WDATA7,
> >  	PWRAP_SPMINF_STA,
> >  	PWRAP_CIPHER_EN,
> > +
> > +	/* MT6797 series regs */
> > +	PWRAP_INT1_EN,
> > +	PWRAP_INT1_FLG_RAW,
> > +	PWRAP_INT1_FLG,
> > +	PWRAP_INT1_CLR,
> >  };
> >  
> >  static int mt2701_regs[] = {
> > @@ -372,6 +378,43 @@ static int mt2701_regs[] = {
> >  	[PWRAP_ADC_RDATA_ADDR2] =	0x154,
> >  };
> >  
> > +static int mt6797_regs[] = {
> > +	[PWRAP_MUX_SEL] =		0x0,
> > +	[PWRAP_WRAP_EN] =		0x4,
> > +	[PWRAP_DIO_EN] =		0x8,
> > +	[PWRAP_SIDLY] =			0xC,
> > +	[PWRAP_RDDMY] =			0x10,
> > +	[PWRAP_CSHEXT_WRITE] =		0x18,
> > +	[PWRAP_CSHEXT_READ] =		0x1C,
> > +	[PWRAP_CSLEXT_START] =		0x20,
> > +	[PWRAP_CSLEXT_END] =		0x24,
> > +	[PWRAP_STAUPD_PRD] =		0x28,
> > +	[PWRAP_HARB_HPRIO] =		0x50,
> > +	[PWRAP_HIPRIO_ARB_EN] =		0x54,
> > +	[PWRAP_MAN_EN] =		0x60,
> > +	[PWRAP_MAN_CMD] =		0x64,
> > +	[PWRAP_WACS0_EN] =		0x70,
> > +	[PWRAP_WACS1_EN] =		0x84,
> > +	[PWRAP_WACS2_EN] =		0x98,
> > +	[PWRAP_INIT_DONE2] =		0x9C,
> > +	[PWRAP_WACS2_CMD] =		0xA0,
> > +	[PWRAP_WACS2_RDATA] =		0xA4,
> > +	[PWRAP_WACS2_VLDCLR] =		0xA8,
> > +	[PWRAP_INT_EN] =		0xC0,
> > +	[PWRAP_INT_FLG_RAW] =		0xC4,
> > +	[PWRAP_INT_FLG] =		0xC8,
> > +	[PWRAP_INT_CLR] =		0xCC,
> > +	[PWRAP_INT1_EN] =		0xD0,
> > +	[PWRAP_INT1_FLG_RAW] =		0xD4,
> > +	[PWRAP_INT1_FLG] =		0xD8,
> > +	[PWRAP_INT1_CLR] =		0xDC,
> > +	[PWRAP_TIMER_EN] =		0xF4,
> > +	[PWRAP_WDT_UNIT] =		0xFC,
> > +	[PWRAP_WDT_SRC_EN] =		0x100,
> > +	[PWRAP_DCM_EN] =		0x1CC,
> > +	[PWRAP_DCM_DBC_PRD] =		0x1D4,
> > +};
> > +
> 
> trim unused registers if any
I removed INT1 related registers.
> 
> >  static int mt7622_regs[] = {
> >  	[PWRAP_MUX_SEL] =		0x0,
> >  	[PWRAP_WRAP_EN] =		0x4,
> > @@ -647,6 +690,7 @@ enum pmic_type {
> >  
> >  enum pwrap_type {
> >  	PWRAP_MT2701,
> > +	PWRAP_MT6797,
> >  	PWRAP_MT7622,
> >  	PWRAP_MT8135,
> >  	PWRAP_MT8173,
> > @@ -1006,6 +1050,12 @@ static void pwrap_init_chip_select_ext(struct pmic_wrapper *wrp, u8 hext_write,
> >  static int pwrap_common_init_reg_clock(struct pmic_wrapper *wrp)
> >  {
> >  	switch (wrp->master->type) {
> > +	case PWRAP_MT6797:
> > +		pwrap_writel(wrp, 0x8, PWRAP_RDDMY);
> > +		pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_RDDMY_NO],
> > +			    0x8);
> > +		pwrap_init_chip_select_ext(wrp, 0x88, 0x55, 3, 0);
> > +		break;
> 
> the setup for timing is much similar to mt2701 + mt6323
> 
> so we can merge the both logic into one, and then hope to eliminate specific pwrap_mt2701_init_reg_clock totally
hmm, we can only keep pwrap_init_chip_select_ext(wrp, 0x88, 0x55, 3, 0);
here.
> 
> >  	case PWRAP_MT8173:
> >  		pwrap_init_chip_select_ext(wrp, 0, 4, 2, 2);
> >  		break;
> > @@ -1076,11 +1126,14 @@ static int pwrap_init_cipher(struct pmic_wrapper *wrp)
> >  		break;
> >  	case PWRAP_MT2701:
> >  	case PWRAP_MT8173:
> > +	case PWRAP_MT6797:
> 
> need to be listed in alphabetical order
no problem
> 
> >  		pwrap_writel(wrp, 1, PWRAP_CIPHER_EN);
> >  		break;
> >  	case PWRAP_MT7622:
> >  		pwrap_writel(wrp, 0, PWRAP_CIPHER_EN);
> >  		break;
> > +	default:
> > +		break;
> >  	}
> >  
> >  	/* Config cipher mode @PMIC */
> > @@ -1325,6 +1378,15 @@ static irqreturn_t pwrap_interrupt(int irqno, void *dev_id)
> >  
> >  	pwrap_writel(wrp, 0xffffffff, PWRAP_INT_CLR);
> >  
> > +	/* If we support INT1 interrupt, we also need to clear it */
> > +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_INT1_EN)) {
> > +		rdata = pwrap_readl(wrp, PWRAP_INT1_FLG);
> > +
> > +		dev_err(wrp->dev, "unexpected interrupt int1=0x%x\n", rdata);
> > +
> > +		pwrap_writel(wrp, rdata, PWRAP_INT1_CLR);
> > +	}
> > +
> 
> it seems no required to add PWRAP_CAP_INT1_EN:
> 
> the CAP is not to take any action for any INTs raised, instead, it can be disabled at initial.

hmm, I think it's fine without adding PWRAP_CAP_INT1_EN.
> 
> >  	return IRQ_HANDLED;
> >  }
> >  
> > @@ -1446,6 +1508,19 @@ static const struct pmic_wrapper_type pwrap_mt8173 = {
> >  	.init_soc_specific = pwrap_mt8173_init_soc_specific,
> >  };
> >  
> > +static const struct pmic_wrapper_type pwrap_mt6797 = {
> > +	.regs = mt6797_regs,
> > +	.type = PWRAP_MT6797,
> > +	.arb_en_all = 0x01fff,
> > +	.int_en_all = 0xffffffc6,
> > +	.int1_en_all = 0x0001ffff,
> > +	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
> > +	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
> > +	.caps = PWRAP_CAP_DCM | PWRAP_CAP_INT1_EN,
> > +	.init_reg_clock = pwrap_common_init_reg_clock,
> > +	.init_soc_specific = NULL,
> > +};
> > +
> >  static const struct of_device_id of_pwrap_match_tbl[] = {
> >  	{
> >  		.compatible = "mediatek,mt2701-pwrap",
> > @@ -1460,6 +1535,9 @@ static const struct of_device_id of_pwrap_match_tbl[] = {
> >  		.compatible = "mediatek,mt8173-pwrap",
> >  		.data = &pwrap_mt8173,
> >  	}, {
> > +		.compatible = "mediatek,mt6797-pwrap",
> > +		.data = &pwrap_mt6797,
> 
> each entry needs to be sorted in alphabetical order
no problem
> 
> > +	}, {
> >  		/* sentinel */
> >  	}
> >  };
> > @@ -1503,11 +1581,13 @@ static int pwrap_probe(struct platform_device *pdev)
> >  	if (IS_ERR(wrp->base))
> >  		return PTR_ERR(wrp->base);
> >  
> > -	wrp->rstc = devm_reset_control_get(wrp->dev, "pwrap");
> > -	if (IS_ERR(wrp->rstc)) {
> > -		ret = PTR_ERR(wrp->rstc);
> > -		dev_dbg(wrp->dev, "cannot get pwrap reset: %d\n", ret);
> > -		return ret;
> > +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_RESET)) {
> > +		wrp->rstc = devm_reset_control_get(wrp->dev, "pwrap");
> 
> there should be a reset bit present for pwrap on infrasys.
> 
> the specific condition can be dropped when the reset cell is exported from infrasys and then the device has a reference to it.
hmm, I think it need to keep here.
because after pwrap initialized, it can't be reset alone.
It needs to reset PMIC simultaneously, too.
> 
> > +		if (IS_ERR(wrp->rstc)) {
> > +			ret = PTR_ERR(wrp->rstc);
> > +			dev_dbg(wrp->dev, "cannot get pwrap reset: %d\n", ret);
> > +			return ret;
> > +		}
> >  	}
> >  
> >  	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_BRIDGE)) {
> > @@ -1549,9 +1629,17 @@ static int pwrap_probe(struct platform_device *pdev)
> >  	if (ret)
> >  		goto err_out1;
> >  
> > -	/* Enable internal dynamic clock */
> > -	pwrap_writel(wrp, 1, PWRAP_DCM_EN);
> > -	pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
> > +	/*
> > +	 * add dcm capability check
> > +	 */
> > +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_DCM)) {
> 
> the specific condition can be dropped since so far all devices the driver can support are owning PWRAP_CAP_DCM
We did not support DCM for future chips.
MT6797 is the last one.
This why I want to add judgement here.
> 
> > +		if (wrp->master->type == PWRAP_MT6797)
> > +			pwrap_writel(wrp, 3, PWRAP_DCM_EN);
> 
> the setup for MT6797 can be moved into .init_soc_specific callback ?

I think put it here is more generally.
> 
> > +		else
> > +			pwrap_writel(wrp, 1, PWRAP_DCM_EN);
> > +
> > +		pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
> > +	}
> >  
> >  	/*
> >  	 * The PMIC could already be initialized by the bootloader.
> > @@ -1580,6 +1668,12 @@ static int pwrap_probe(struct platform_device *pdev)
> >  	pwrap_writel(wrp, wrp->master->wdt_src, PWRAP_WDT_SRC_EN);
> >  	pwrap_writel(wrp, 0x1, PWRAP_TIMER_EN);
> >  	pwrap_writel(wrp, wrp->master->int_en_all, PWRAP_INT_EN);
> > +	/*
> > +	 * We add INT1 interrupt to handle starvation and request exception
> > +	 * If we support it, we should enable them here.
> > +	 */
> > +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_INT1_EN))
> > +		pwrap_writel(wrp, wrp->master->int1_en_all, PWRAP_INT1_EN);
> >  
> 
> if there is no explicitly enabling on INT1, then ISR handling for INT1 is also unnecessary

It's ok for me.
> 
> >  	irq = platform_get_irq(pdev, 0);
> >  	ret = devm_request_irq(wrp->dev, irq, pwrap_interrupt,
> 
> 
> 

Re: [PATCH V4 5/8] soc: mediatek: pwrap: add pwrap for mt6797 SoCs

[Sean Wang]

On Thu, 2018-05-03 at 14:20 +0800, Argus Lin wrote:
> On Thu, 2018-05-03 at 12:01 +0800, Sean Wang wrote:
> >  };

[...]

> > > @@ -1503,11 +1581,13 @@ static int pwrap_probe(struct platform_device *pdev)
> > >  	if (IS_ERR(wrp->base))
> > >  		return PTR_ERR(wrp->base);
> > >  
> > > -	wrp->rstc = devm_reset_control_get(wrp->dev, "pwrap");
> > > -	if (IS_ERR(wrp->rstc)) {
> > > -		ret = PTR_ERR(wrp->rstc);
> > > -		dev_dbg(wrp->dev, "cannot get pwrap reset: %d\n", ret);
> > > -		return ret;
> > > +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_RESET)) {
> > > +		wrp->rstc = devm_reset_control_get(wrp->dev, "pwrap");
> > 
> > there should be a reset bit present for pwrap on infrasys.
> > 
> > the specific condition can be dropped when the reset cell is exported from infrasys and then the device has a reference to it.
> hmm, I think it need to keep here.
> because after pwrap initialized, it can't be reset alone.
> It needs to reset PMIC simultaneously, too.

Reset a pair, either a master or its slave, all had been a part of
pwrap_init.

The reset controller provided here is just to reset pwrap device.
And for its slave reset, it should be done by pwrap_reset_spislave. 

So for MT6397, it should be able to fall into the same procedure.

> > 
> > > +		if (IS_ERR(wrp->rstc)) {
> > > +			ret = PTR_ERR(wrp->rstc);
> > > +			dev_dbg(wrp->dev, "cannot get pwrap reset: %d\n", ret);
> > > +			return ret;
> > > +		}
> > >  	}
> > >  
> > >  	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_BRIDGE)) {
> > > @@ -1549,9 +1629,17 @@ static int pwrap_probe(struct platform_device *pdev)
> > >  	if (ret)
> > >  		goto err_out1;
> > >  
> > > -	/* Enable internal dynamic clock */
> > > -	pwrap_writel(wrp, 1, PWRAP_DCM_EN);
> > > -	pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
> > > +	/*
> > > +	 * add dcm capability check
> > > +	 */
> > > +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_DCM)) {
> > 
> > the specific condition can be dropped since so far all devices the driver can support are owning PWRAP_CAP_DCM
> We did not support DCM for future chips.
> MT6797 is the last one.
> This why I want to add judgement here.

The series is only for MT6797 pwrap, so it's fine with only adding these
things the SoC actually relies on. 

PWRAP_CAP_DCM should not be added until a new SoC without dcm is really
introduced.

> > 
> > > +		if (wrp->master->type == PWRAP_MT6797)
> > > +			pwrap_writel(wrp, 3, PWRAP_DCM_EN);
> > 
> > the setup for MT6797 can be moved into .init_soc_specific callback ?
> 
> I think put it here is more generally.
> > 
> > > +		else
> > > +			pwrap_writel(wrp, 1, PWRAP_DCM_EN);
> > > +
> > > +		pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
> > > +	}
> > >  
> > >  	/*
> > >  	 * The PMIC could already be initialized by the bootloader.
> > > @@ -1580,6 +1668,12 @@ static int pwrap_probe(struct platform_device *pdev)
> > >  	pwrap_writel(wrp, wrp->master->wdt_src, PWRAP_WDT_SRC_EN);
> > >  	pwrap_writel(wrp, 0x1, PWRAP_TIMER_EN);
> > >  	pwrap_writel(wrp, wrp->master->int_en_all, PWRAP_INT_EN);
> > > +	/*
> > > +	 * We add INT1 interrupt to handle starvation and request exception
> > > +	 * If we support it, we should enable them here.
> > > +	 */
> > > +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_INT1_EN))
> > > +		pwrap_writel(wrp, wrp->master->int1_en_all, PWRAP_INT1_EN);
> > >  
> > 
> > if there is no explicitly enabling on INT1, then ISR handling for INT1 is also unnecessary
> 
> It's ok for me.
> > 
> > >  	irq = platform_get_irq(pdev, 0);
> > >  	ret = devm_request_irq(wrp->dev, irq, pwrap_interrupt,
> > 
> > 
> > 
> 
> 

Re: [PATCH V4 5/8] soc: mediatek: pwrap: add pwrap for mt6797 SoCs

[Sean Wang]

On Thu, 2018-05-03 at 14:20 +0800, Argus Lin wrote:
> On Thu, 2018-05-03 at 12:01 +0800, Sean Wang wrote:
> >  };

[...]

> > > @@ -1503,11 +1581,13 @@ static int pwrap_probe(struct platform_device *pdev)
> > >  	if (IS_ERR(wrp->base))
> > >  		return PTR_ERR(wrp->base);
> > >  
> > > -	wrp->rstc = devm_reset_control_get(wrp->dev, "pwrap");
> > > -	if (IS_ERR(wrp->rstc)) {
> > > -		ret = PTR_ERR(wrp->rstc);
> > > -		dev_dbg(wrp->dev, "cannot get pwrap reset: %d\n", ret);
> > > -		return ret;
> > > +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_RESET)) {
> > > +		wrp->rstc = devm_reset_control_get(wrp->dev, "pwrap");
> > 
> > there should be a reset bit present for pwrap on infrasys.
> > 
> > the specific condition can be dropped when the reset cell is exported from infrasys and then the device has a reference to it.
> hmm, I think it need to keep here.
> because after pwrap initialized, it can't be reset alone.
> It needs to reset PMIC simultaneously, too.

Reset a pair, either a master or its slave, all had been a part of
pwrap_init.

The reset controller provided here is just to reset pwrap device.
And for its slave reset, it should be done by pwrap_reset_spislave. 

So for MT6397, it should be able to fall into the same procedure.

> > 
> > > +		if (IS_ERR(wrp->rstc)) {
> > > +			ret = PTR_ERR(wrp->rstc);
> > > +			dev_dbg(wrp->dev, "cannot get pwrap reset: %d\n", ret);
> > > +			return ret;
> > > +		}
> > >  	}
> > >  
> > >  	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_BRIDGE)) {
> > > @@ -1549,9 +1629,17 @@ static int pwrap_probe(struct platform_device *pdev)
> > >  	if (ret)
> > >  		goto err_out1;
> > >  
> > > -	/* Enable internal dynamic clock */
> > > -	pwrap_writel(wrp, 1, PWRAP_DCM_EN);
> > > -	pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
> > > +	/*
> > > +	 * add dcm capability check
> > > +	 */
> > > +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_DCM)) {
> > 
> > the specific condition can be dropped since so far all devices the driver can support are owning PWRAP_CAP_DCM
> We did not support DCM for future chips.
> MT6797 is the last one.
> This why I want to add judgement here.

The series is only for MT6797 pwrap, so it's fine with only adding these
things the SoC actually relies on. 

PWRAP_CAP_DCM should not be added until a new SoC without dcm is really
introduced.

> > 
> > > +		if (wrp->master->type == PWRAP_MT6797)
> > > +			pwrap_writel(wrp, 3, PWRAP_DCM_EN);
> > 
> > the setup for MT6797 can be moved into .init_soc_specific callback ?
> 
> I think put it here is more generally.
> > 
> > > +		else
> > > +			pwrap_writel(wrp, 1, PWRAP_DCM_EN);
> > > +
> > > +		pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
> > > +	}
> > >  
> > >  	/*
> > >  	 * The PMIC could already be initialized by the bootloader.
> > > @@ -1580,6 +1668,12 @@ static int pwrap_probe(struct platform_device *pdev)
> > >  	pwrap_writel(wrp, wrp->master->wdt_src, PWRAP_WDT_SRC_EN);
> > >  	pwrap_writel(wrp, 0x1, PWRAP_TIMER_EN);
> > >  	pwrap_writel(wrp, wrp->master->int_en_all, PWRAP_INT_EN);
> > > +	/*
> > > +	 * We add INT1 interrupt to handle starvation and request exception
> > > +	 * If we support it, we should enable them here.
> > > +	 */
> > > +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_INT1_EN))
> > > +		pwrap_writel(wrp, wrp->master->int1_en_all, PWRAP_INT1_EN);
> > >  
> > 
> > if there is no explicitly enabling on INT1, then ISR handling for INT1 is also unnecessary
> 
> It's ok for me.
> > 
> > >  	irq = platform_get_irq(pdev, 0);
> > >  	ret = devm_request_irq(wrp->dev, irq, pwrap_interrupt,
> > 
> > 
> > 
> 
> 

[PATCH V4 5/8] soc: mediatek: pwrap: add pwrap for mt6797 SoCs

[Sean Wang]

On Thu, 2018-05-03 at 14:20 +0800, Argus Lin wrote:
> On Thu, 2018-05-03 at 12:01 +0800, Sean Wang wrote:
> >  };

[...]

> > > @@ -1503,11 +1581,13 @@ static int pwrap_probe(struct platform_device *pdev)
> > >  	if (IS_ERR(wrp->base))
> > >  		return PTR_ERR(wrp->base);
> > >  
> > > -	wrp->rstc = devm_reset_control_get(wrp->dev, "pwrap");
> > > -	if (IS_ERR(wrp->rstc)) {
> > > -		ret = PTR_ERR(wrp->rstc);
> > > -		dev_dbg(wrp->dev, "cannot get pwrap reset: %d\n", ret);
> > > -		return ret;
> > > +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_RESET)) {
> > > +		wrp->rstc = devm_reset_control_get(wrp->dev, "pwrap");
> > 
> > there should be a reset bit present for pwrap on infrasys.
> > 
> > the specific condition can be dropped when the reset cell is exported from infrasys and then the device has a reference to it.
> hmm, I think it need to keep here.
> because after pwrap initialized, it can't be reset alone.
> It needs to reset PMIC simultaneously, too.

Reset a pair, either a master or its slave, all had been a part of
pwrap_init.

The reset controller provided here is just to reset pwrap device.
And for its slave reset, it should be done by pwrap_reset_spislave. 

So for MT6397, it should be able to fall into the same procedure.

> > 
> > > +		if (IS_ERR(wrp->rstc)) {
> > > +			ret = PTR_ERR(wrp->rstc);
> > > +			dev_dbg(wrp->dev, "cannot get pwrap reset: %d\n", ret);
> > > +			return ret;
> > > +		}
> > >  	}
> > >  
> > >  	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_BRIDGE)) {
> > > @@ -1549,9 +1629,17 @@ static int pwrap_probe(struct platform_device *pdev)
> > >  	if (ret)
> > >  		goto err_out1;
> > >  
> > > -	/* Enable internal dynamic clock */
> > > -	pwrap_writel(wrp, 1, PWRAP_DCM_EN);
> > > -	pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
> > > +	/*
> > > +	 * add dcm capability check
> > > +	 */
> > > +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_DCM)) {
> > 
> > the specific condition can be dropped since so far all devices the driver can support are owning PWRAP_CAP_DCM
> We did not support DCM for future chips.
> MT6797 is the last one.
> This why I want to add judgement here.

The series is only for MT6797 pwrap, so it's fine with only adding these
things the SoC actually relies on. 

PWRAP_CAP_DCM should not be added until a new SoC without dcm is really
introduced.

> > 
> > > +		if (wrp->master->type == PWRAP_MT6797)
> > > +			pwrap_writel(wrp, 3, PWRAP_DCM_EN);
> > 
> > the setup for MT6797 can be moved into .init_soc_specific callback ?
> 
> I think put it here is more generally.
> > 
> > > +		else
> > > +			pwrap_writel(wrp, 1, PWRAP_DCM_EN);
> > > +
> > > +		pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
> > > +	}
> > >  
> > >  	/*
> > >  	 * The PMIC could already be initialized by the bootloader.
> > > @@ -1580,6 +1668,12 @@ static int pwrap_probe(struct platform_device *pdev)
> > >  	pwrap_writel(wrp, wrp->master->wdt_src, PWRAP_WDT_SRC_EN);
> > >  	pwrap_writel(wrp, 0x1, PWRAP_TIMER_EN);
> > >  	pwrap_writel(wrp, wrp->master->int_en_all, PWRAP_INT_EN);
> > > +	/*
> > > +	 * We add INT1 interrupt to handle starvation and request exception
> > > +	 * If we support it, we should enable them here.
> > > +	 */
> > > +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_INT1_EN))
> > > +		pwrap_writel(wrp, wrp->master->int1_en_all, PWRAP_INT1_EN);
> > >  
> > 
> > if there is no explicitly enabling on INT1, then ISR handling for INT1 is also unnecessary
> 
> It's ok for me.
> > 
> > >  	irq = platform_get_irq(pdev, 0);
> > >  	ret = devm_request_irq(wrp->dev, irq, pwrap_interrupt,
> > 
> > 
> > 
> 
> 

Re: [PATCH V4 5/8] soc: mediatek: pwrap: add pwrap for mt6797 SoCs

[Matthias Brugger]

On 05/04/2018 05:04 AM, Sean Wang wrote:
> On Thu, 2018-05-03 at 14:20 +0800, Argus Lin wrote:
>> On Thu, 2018-05-03 at 12:01 +0800, Sean Wang wrote:
>>>  };
> 
> [...]
> 
>>>> @@ -1503,11 +1581,13 @@ static int pwrap_probe(struct platform_device *pdev)
>>>>  	if (IS_ERR(wrp->base))
>>>>  		return PTR_ERR(wrp->base);
>>>>  
>>>> -	wrp->rstc = devm_reset_control_get(wrp->dev, "pwrap");
>>>> -	if (IS_ERR(wrp->rstc)) {
>>>> -		ret = PTR_ERR(wrp->rstc);
>>>> -		dev_dbg(wrp->dev, "cannot get pwrap reset: %d\n", ret);
>>>> -		return ret;
>>>> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_RESET)) {
>>>> +		wrp->rstc = devm_reset_control_get(wrp->dev, "pwrap");
>>>
>>> there should be a reset bit present for pwrap on infrasys.
>>>
>>> the specific condition can be dropped when the reset cell is exported from infrasys and then the device has a reference to it.
>> hmm, I think it need to keep here.
>> because after pwrap initialized, it can't be reset alone.
>> It needs to reset PMIC simultaneously, too.
> 
> Reset a pair, either a master or its slave, all had been a part of
> pwrap_init.
> 
> The reset controller provided here is just to reset pwrap device.
> And for its slave reset, it should be done by pwrap_reset_spislave. 
> 
> So for MT6397, it should be able to fall into the same procedure.
> 
>>>
>>>> +		if (IS_ERR(wrp->rstc)) {
>>>> +			ret = PTR_ERR(wrp->rstc);
>>>> +			dev_dbg(wrp->dev, "cannot get pwrap reset: %d\n", ret);
>>>> +			return ret;
>>>> +		}
>>>>  	}
>>>>  
>>>>  	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_BRIDGE)) {
>>>> @@ -1549,9 +1629,17 @@ static int pwrap_probe(struct platform_device *pdev)
>>>>  	if (ret)
>>>>  		goto err_out1;
>>>>  
>>>> -	/* Enable internal dynamic clock */
>>>> -	pwrap_writel(wrp, 1, PWRAP_DCM_EN);
>>>> -	pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
>>>> +	/*
>>>> +	 * add dcm capability check
>>>> +	 */
>>>> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_DCM)) {
>>>
>>> the specific condition can be dropped since so far all devices the driver can support are owning PWRAP_CAP_DCM
>> We did not support DCM for future chips.
>> MT6797 is the last one.
>> This why I want to add judgement here.
> 
> The series is only for MT6797 pwrap, so it's fine with only adding these
> things the SoC actually relies on. 
> 
> PWRAP_CAP_DCM should not be added until a new SoC without dcm is really
> introduced.
> 

I agree (and I think I said this already in a previous review).

Regards,
Matthias

>>>
>>>> +		if (wrp->master->type == PWRAP_MT6797)
>>>> +			pwrap_writel(wrp, 3, PWRAP_DCM_EN);
>>>
>>> the setup for MT6797 can be moved into .init_soc_specific callback ?
>>
>> I think put it here is more generally.
>>>
>>>> +		else
>>>> +			pwrap_writel(wrp, 1, PWRAP_DCM_EN);
>>>> +
>>>> +		pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
>>>> +	}
>>>>  
>>>>  	/*
>>>>  	 * The PMIC could already be initialized by the bootloader.
>>>> @@ -1580,6 +1668,12 @@ static int pwrap_probe(struct platform_device *pdev)
>>>>  	pwrap_writel(wrp, wrp->master->wdt_src, PWRAP_WDT_SRC_EN);
>>>>  	pwrap_writel(wrp, 0x1, PWRAP_TIMER_EN);
>>>>  	pwrap_writel(wrp, wrp->master->int_en_all, PWRAP_INT_EN);
>>>> +	/*
>>>> +	 * We add INT1 interrupt to handle starvation and request exception
>>>> +	 * If we support it, we should enable them here.
>>>> +	 */
>>>> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_INT1_EN))
>>>> +		pwrap_writel(wrp, wrp->master->int1_en_all, PWRAP_INT1_EN);
>>>>  
>>>
>>> if there is no explicitly enabling on INT1, then ISR handling for INT1 is also unnecessary
>>
>> It's ok for me.
>>>
>>>>  	irq = platform_get_irq(pdev, 0);
>>>>  	ret = devm_request_irq(wrp->dev, irq, pwrap_interrupt,
>>>
>>>
>>>
>>
>>
> 
> 

[PATCH V4 5/8] soc: mediatek: pwrap: add pwrap for mt6797 SoCs

[Matthias Brugger]

On 05/04/2018 05:04 AM, Sean Wang wrote:
> On Thu, 2018-05-03 at 14:20 +0800, Argus Lin wrote:
>> On Thu, 2018-05-03 at 12:01 +0800, Sean Wang wrote:
>>>  };
> 
> [...]
> 
>>>> @@ -1503,11 +1581,13 @@ static int pwrap_probe(struct platform_device *pdev)
>>>>  	if (IS_ERR(wrp->base))
>>>>  		return PTR_ERR(wrp->base);
>>>>  
>>>> -	wrp->rstc = devm_reset_control_get(wrp->dev, "pwrap");
>>>> -	if (IS_ERR(wrp->rstc)) {
>>>> -		ret = PTR_ERR(wrp->rstc);
>>>> -		dev_dbg(wrp->dev, "cannot get pwrap reset: %d\n", ret);
>>>> -		return ret;
>>>> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_RESET)) {
>>>> +		wrp->rstc = devm_reset_control_get(wrp->dev, "pwrap");
>>>
>>> there should be a reset bit present for pwrap on infrasys.
>>>
>>> the specific condition can be dropped when the reset cell is exported from infrasys and then the device has a reference to it.
>> hmm, I think it need to keep here.
>> because after pwrap initialized, it can't be reset alone.
>> It needs to reset PMIC simultaneously, too.
> 
> Reset a pair, either a master or its slave, all had been a part of
> pwrap_init.
> 
> The reset controller provided here is just to reset pwrap device.
> And for its slave reset, it should be done by pwrap_reset_spislave. 
> 
> So for MT6397, it should be able to fall into the same procedure.
> 
>>>
>>>> +		if (IS_ERR(wrp->rstc)) {
>>>> +			ret = PTR_ERR(wrp->rstc);
>>>> +			dev_dbg(wrp->dev, "cannot get pwrap reset: %d\n", ret);
>>>> +			return ret;
>>>> +		}
>>>>  	}
>>>>  
>>>>  	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_BRIDGE)) {
>>>> @@ -1549,9 +1629,17 @@ static int pwrap_probe(struct platform_device *pdev)
>>>>  	if (ret)
>>>>  		goto err_out1;
>>>>  
>>>> -	/* Enable internal dynamic clock */
>>>> -	pwrap_writel(wrp, 1, PWRAP_DCM_EN);
>>>> -	pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
>>>> +	/*
>>>> +	 * add dcm capability check
>>>> +	 */
>>>> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_DCM)) {
>>>
>>> the specific condition can be dropped since so far all devices the driver can support are owning PWRAP_CAP_DCM
>> We did not support DCM for future chips.
>> MT6797 is the last one.
>> This why I want to add judgement here.
> 
> The series is only for MT6797 pwrap, so it's fine with only adding these
> things the SoC actually relies on. 
> 
> PWRAP_CAP_DCM should not be added until a new SoC without dcm is really
> introduced.
> 

I agree (and I think I said this already in a previous review).

Regards,
Matthias

>>>
>>>> +		if (wrp->master->type == PWRAP_MT6797)
>>>> +			pwrap_writel(wrp, 3, PWRAP_DCM_EN);
>>>
>>> the setup for MT6797 can be moved into .init_soc_specific callback ?
>>
>> I think put it here is more generally.
>>>
>>>> +		else
>>>> +			pwrap_writel(wrp, 1, PWRAP_DCM_EN);
>>>> +
>>>> +		pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
>>>> +	}
>>>>  
>>>>  	/*
>>>>  	 * The PMIC could already be initialized by the bootloader.
>>>> @@ -1580,6 +1668,12 @@ static int pwrap_probe(struct platform_device *pdev)
>>>>  	pwrap_writel(wrp, wrp->master->wdt_src, PWRAP_WDT_SRC_EN);
>>>>  	pwrap_writel(wrp, 0x1, PWRAP_TIMER_EN);
>>>>  	pwrap_writel(wrp, wrp->master->int_en_all, PWRAP_INT_EN);
>>>> +	/*
>>>> +	 * We add INT1 interrupt to handle starvation and request exception
>>>> +	 * If we support it, we should enable them here.
>>>> +	 */
>>>> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_INT1_EN))
>>>> +		pwrap_writel(wrp, wrp->master->int1_en_all, PWRAP_INT1_EN);
>>>>  
>>>
>>> if there is no explicitly enabling on INT1, then ISR handling for INT1 is also unnecessary
>>
>> It's ok for me.
>>>
>>>>  	irq = platform_get_irq(pdev, 0);
>>>>  	ret = devm_request_irq(wrp->dev, irq, pwrap_interrupt,
>>>
>>>
>>>
>>
>>
> 
>