[PATCH v4 0/7] Add PMIC support to MediaTek MT7622 SoC

[PATCH v4 0/7] Add PMIC support to MediaTek MT7622 SoC

[sean.wang-NuS5LvNUpcJWk0Htik3J/w]

From: Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>

Changes since v3
- rebase into Linux 4.14-rc1
- removed those patches already applied

Changes since v2:
- for patch 1/2, enhance the document as the suggestions from v2.
- for patch 3, constify the table with struct regulator_ops, also fix that
regulator doesn't have modesetting facilities it just shouldn't have the ops
and finally fix copy+paste mistake in warranty disclaimer.
- for patch 7/8, sort the things added for MT7622/MT6380 alphabetically.

Chenglin Xu (1):
  soc: mediatek: pwrap: add support for MT7622 SoC

Sean Wang (6):
  dt-bindings: arm: mediatek: add MT7622 string to the PMIC wrapper doc
  soc: mediatek: pwrap: add pwrap_read32 for reading in 32-bit mode
  soc: mediatek: pwrap: add pwrap_write32 for writing in 32-bit mode
  soc: mediatek: pwrap: update pwrap_init without slave programming
  soc: mediatek: pwrap: add MediaTek MT6380 as one slave of pwrap
  soc: mediatek: pwrap: fixup warnings from coding style

 .../devicetree/bindings/soc/mediatek/pwrap.txt     |   1 +
 drivers/soc/mediatek/mtk-pmic-wrap.c               | 417 ++++++++++++++++++---
 2 files changed, 362 insertions(+), 56 deletions(-)

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 1/7] dt-bindings: arm: mediatek: add MT7622 string to the PMIC wrapper doc

[sean.wang]

From: Sean Wang <sean.wang@mediatek.com>

Signed-off-by: Chenglin Xu <chenglin.xu@mediatek.com>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 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 107700d..bd97f22 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,mt7622-pwrap" for MT7622 SoCs
 	"mediatek,mt8135-pwrap" for MT8135 SoCs
 	"mediatek,mt8173-pwrap" for MT8173 SoCs
 - interrupts: IRQ for pwrap in SOC
-- 
2.7.4

[PATCH v4 2/7] soc: mediatek: pwrap: add pwrap_read32 for reading in 32-bit mode

[sean.wang-NuS5LvNUpcJWk0Htik3J/w]

From: Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>

Some regulators such as MediaTek MT6380 has to be read in 32-bit mode.
So the patch adds pwrap_read32, rename old pwrap_read into pwrap_read16
and one function pointer is introduced for increasing flexibility allowing
the determination which mode is used by the pwrap slave detection through
device tree.

Signed-off-by: Chenglin Xu <chenglin.xu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
Signed-off-by: Chen Zhong <chen.zhong-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
Signed-off-by: Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 drivers/soc/mediatek/mtk-pmic-wrap.c | 54 +++++++++++++++++++++++++++++++++++-
 1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index c204838..db55f13 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -487,6 +487,7 @@ static int mt8135_regs[] = {
 
 enum pmic_type {
 	PMIC_MT6323,
+	PMIC_MT6380,
 	PMIC_MT6397,
 };
 
@@ -496,9 +497,15 @@ enum pwrap_type {
 	PWRAP_MT8173,
 };
 
+struct pmic_wrapper;
 struct pwrap_slv_type {
 	const u32 *dew_regs;
 	enum pmic_type type;
+	/* pwrap operations are highly associated with the PMIC types,
+	 * so the pointers added increases flexibility allowing determination
+	 * which type is used by the detection through device tree.
+	 */
+	int (*pwrap_read)(struct pmic_wrapper *wrp, u32 adr, u32 *rdata);
 };
 
 struct pmic_wrapper {
@@ -609,7 +616,7 @@ static int pwrap_write(struct pmic_wrapper *wrp, u32 adr, u32 wdata)
 	return 0;
 }
 
-static int pwrap_read(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)
+static int pwrap_read16(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)
 {
 	int ret;
 
@@ -632,6 +639,39 @@ static int pwrap_read(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)
 	return 0;
 }
 
+static int pwrap_read32(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)
+{
+	int ret, msb;
+
+	*rdata = 0;
+	for (msb = 0; msb < 2; msb++) {
+		ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_idle);
+		if (ret) {
+			pwrap_leave_fsm_vldclr(wrp);
+			return ret;
+		}
+
+		pwrap_writel(wrp, ((msb << 30) | (adr << 16)),
+			     PWRAP_WACS2_CMD);
+
+		ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_vldclr);
+		if (ret)
+			return ret;
+
+		*rdata += (PWRAP_GET_WACS_RDATA(pwrap_readl(wrp,
+			   PWRAP_WACS2_RDATA)) << (16 * msb));
+
+		pwrap_writel(wrp, 1, PWRAP_WACS2_VLDCLR);
+	}
+
+	return 0;
+}
+
+static int pwrap_read(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)
+{
+	return wrp->slave->pwrap_read(wrp, adr, rdata);
+}
+
 static int pwrap_regmap_read(void *context, u32 adr, u32 *rdata)
 {
 	return pwrap_read(context, adr, rdata);
@@ -752,6 +792,8 @@ static int pwrap_mt2701_init_reg_clock(struct pmic_wrapper *wrp)
 		pwrap_writel(wrp, 0x2, PWRAP_CSLEXT_START);
 		pwrap_writel(wrp, 0x2, PWRAP_CSLEXT_END);
 		break;
+	default:
+		break;
 	}
 
 	return 0;
@@ -815,6 +857,8 @@ static int pwrap_init_cipher(struct pmic_wrapper *wrp)
 		pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_CIPHER_EN],
 			    0x1);
 		break;
+	default:
+		break;
 	}
 
 	/* wait for cipher data ready@AP */
@@ -1035,11 +1079,19 @@ static const struct regmap_config pwrap_regmap_config = {
 static const struct pwrap_slv_type pmic_mt6323 = {
 	.dew_regs = mt6323_regs,
 	.type = PMIC_MT6323,
+	.pwrap_read = pwrap_read16,
+};
+
+static const struct pwrap_slv_type pmic_mt6380 = {
+	.dew_regs = NULL,
+	.type = PMIC_MT6380,
+	.pwrap_read = pwrap_read32,
 };
 
 static const struct pwrap_slv_type pmic_mt6397 = {
 	.dew_regs = mt6397_regs,
 	.type = PMIC_MT6397,
+	.pwrap_read = pwrap_read16,
 };
 
 static const struct of_device_id of_slave_match_tbl[] = {
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 3/7] soc: mediatek: pwrap: add pwrap_write32 for writing in 32-bit mode

[sean.wang]

From: Sean Wang <sean.wang@mediatek.com>

Some regulators such as MediaTek MT6380 also has to be written in
32-bit mode. So the patch adds pwrap_write32, rename old pwrap_write
into pwrap_write16 and one additional function pointer is introduced
for increasing flexibility allowing the determination which mode is
used by the pwrap slave detection through device tree.

Signed-off-by: Chenglin Xu <chenglin.xu@mediatek.com>
Signed-off-by: Chen Zhong <chen.zhong@mediatek.com>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 drivers/soc/mediatek/mtk-pmic-wrap.c | 63 +++++++++++++++++++++++++++---------
 1 file changed, 47 insertions(+), 16 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index db55f13..27d7ccc 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -506,6 +506,7 @@ struct pwrap_slv_type {
 	 * which type is used by the detection through device tree.
 	 */
 	int (*pwrap_read)(struct pmic_wrapper *wrp, u32 adr, u32 *rdata);
+	int (*pwrap_write)(struct pmic_wrapper *wrp, u32 adr, u32 wdata);
 };
 
 struct pmic_wrapper {
@@ -600,22 +601,6 @@ static int pwrap_wait_for_state(struct pmic_wrapper *wrp,
 	} while (1);
 }
 
-static int pwrap_write(struct pmic_wrapper *wrp, u32 adr, u32 wdata)
-{
-	int ret;
-
-	ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_idle);
-	if (ret) {
-		pwrap_leave_fsm_vldclr(wrp);
-		return ret;
-	}
-
-	pwrap_writel(wrp, (1 << 31) | ((adr >> 1) << 16) | wdata,
-			PWRAP_WACS2_CMD);
-
-	return 0;
-}
-
 static int pwrap_read16(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)
 {
 	int ret;
@@ -672,6 +657,49 @@ static int pwrap_read(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)
 	return wrp->slave->pwrap_read(wrp, adr, rdata);
 }
 
+static int pwrap_write16(struct pmic_wrapper *wrp, u32 adr, u32 wdata)
+{
+	int ret;
+
+	ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_idle);
+	if (ret) {
+		pwrap_leave_fsm_vldclr(wrp);
+		return ret;
+	}
+
+	pwrap_writel(wrp, (1 << 31) | ((adr >> 1) << 16) | wdata,
+		     PWRAP_WACS2_CMD);
+
+	return 0;
+}
+
+static int pwrap_write32(struct pmic_wrapper *wrp, u32 adr, u32 wdata)
+{
+	int ret, msb, rdata;
+
+	for (msb = 0; msb < 2; msb++) {
+		ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_idle);
+		if (ret) {
+			pwrap_leave_fsm_vldclr(wrp);
+			return ret;
+		}
+
+		pwrap_writel(wrp, (1 << 31) | (msb << 30) | (adr << 16) |
+			     ((wdata >> (msb * 16)) & 0xffff),
+			     PWRAP_WACS2_CMD);
+
+		if (!msb)
+			pwrap_read(wrp, adr, &rdata);
+	}
+
+	return 0;
+}
+
+static int pwrap_write(struct pmic_wrapper *wrp, u32 adr, u32 wdata)
+{
+	return wrp->slave->pwrap_write(wrp, adr, wdata);
+}
+
 static int pwrap_regmap_read(void *context, u32 adr, u32 *rdata)
 {
 	return pwrap_read(context, adr, rdata);
@@ -1080,18 +1108,21 @@ static const struct pwrap_slv_type pmic_mt6323 = {
 	.dew_regs = mt6323_regs,
 	.type = PMIC_MT6323,
 	.pwrap_read = pwrap_read16,
+	.pwrap_write = pwrap_write16,
 };
 
 static const struct pwrap_slv_type pmic_mt6380 = {
 	.dew_regs = NULL,
 	.type = PMIC_MT6380,
 	.pwrap_read = pwrap_read32,
+	.pwrap_write = pwrap_write32,
 };
 
 static const struct pwrap_slv_type pmic_mt6397 = {
 	.dew_regs = mt6397_regs,
 	.type = PMIC_MT6397,
 	.pwrap_read = pwrap_read16,
+	.pwrap_write = pwrap_write16,
 };
 
 static const struct of_device_id of_slave_match_tbl[] = {
-- 
2.7.4

[PATCH v4 4/7] soc: mediatek: pwrap: update pwrap_init without slave programming

[sean.wang]

From: Sean Wang <sean.wang@mediatek.com>

pwrap initialization is highly associated with the base SoC, so
update here for allowing pwrap_init without slave program which would be
used to those PMICs without extra encryption on bus such as MT6380.

Signed-off-by: Chenglin Xu <chenglin.xu@mediatek.com>
Signed-off-by: Chen Zhong <chen.zhong@mediatek.com>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 drivers/soc/mediatek/mtk-pmic-wrap.c | 91 +++++++++++++++++++++---------------
 1 file changed, 54 insertions(+), 37 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index 27d7ccc..9c6d855 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -531,6 +531,7 @@ struct pmic_wrapper_type {
 	u32 spi_w;
 	u32 wdt_src;
 	int has_bridge:1;
+	int slv_program:1;
 	int (*init_reg_clock)(struct pmic_wrapper *wrp);
 	int (*init_soc_specific)(struct pmic_wrapper *wrp);
 };
@@ -999,9 +1000,12 @@ static int pwrap_init(struct pmic_wrapper *wrp)
 	}
 
 	/* Reset SPI slave */
-	ret = pwrap_reset_spislave(wrp);
-	if (ret)
-		return ret;
+
+	if (wrp->master->slv_program) {
+		ret = pwrap_reset_spislave(wrp);
+		if (ret)
+			return ret;
+	}
 
 	pwrap_writel(wrp, 1, PWRAP_WRAP_EN);
 
@@ -1013,45 +1017,52 @@ static int pwrap_init(struct pmic_wrapper *wrp)
 	if (ret)
 		return ret;
 
-	/* Setup serial input delay */
-	ret = pwrap_init_sidly(wrp);
-	if (ret)
-		return ret;
+	if (wrp->master->slv_program) {
+		/* Setup serial input delay */
+		ret = pwrap_init_sidly(wrp);
+		if (ret)
+			return ret;
 
-	/* Enable dual IO mode */
-	pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_DIO_EN], 1);
+		/* Enable dual IO mode */
+		pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_DIO_EN], 1);
 
-	/* Check IDLE & INIT_DONE in advance */
-	ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_idle_and_sync_idle);
-	if (ret) {
-		dev_err(wrp->dev, "%s fail, ret=%d\n", __func__, ret);
-		return ret;
-	}
+		/* Check IDLE & INIT_DONE in advance */
+		ret = pwrap_wait_for_state(wrp,
+					   pwrap_is_fsm_idle_and_sync_idle);
+		if (ret) {
+			dev_err(wrp->dev, "%s fail, ret=%d\n", __func__, ret);
+			return ret;
+		}
 
-	pwrap_writel(wrp, 1, PWRAP_DIO_EN);
+		pwrap_writel(wrp, 1, PWRAP_DIO_EN);
 
-	/* Read Test */
-	pwrap_read(wrp, wrp->slave->dew_regs[PWRAP_DEW_READ_TEST], &rdata);
-	if (rdata != PWRAP_DEW_READ_TEST_VAL) {
-		dev_err(wrp->dev, "Read test failed after switch to DIO mode: 0x%04x != 0x%04x\n",
+		/* Read Test */
+		pwrap_read(wrp,
+			   wrp->slave->dew_regs[PWRAP_DEW_READ_TEST], &rdata);
+		if (rdata != PWRAP_DEW_READ_TEST_VAL) {
+			dev_err(wrp->dev,
+				"Read failed on DIO mode: 0x%04x!=0x%04x\n",
 				PWRAP_DEW_READ_TEST_VAL, rdata);
-		return -EFAULT;
-	}
-
-	/* Enable encryption */
-	ret = pwrap_init_cipher(wrp);
-	if (ret)
-		return ret;
+			return -EFAULT;
+		}
 
-	/* Signature checking - using CRC */
-	if (pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_CRC_EN], 0x1))
-		return -EFAULT;
+		/* Enable encryption */
+		ret = pwrap_init_cipher(wrp);
+		if (ret)
+			return ret;
 
-	pwrap_writel(wrp, 0x1, PWRAP_CRC_EN);
-	pwrap_writel(wrp, 0x0, PWRAP_SIG_MODE);
-	pwrap_writel(wrp, wrp->slave->dew_regs[PWRAP_DEW_CRC_VAL],
-		     PWRAP_SIG_ADR);
-	pwrap_writel(wrp, wrp->master->arb_en_all, PWRAP_HIPRIO_ARB_EN);
+		/* Signature checking - using CRC */
+		if (pwrap_write(wrp,
+				wrp->slave->dew_regs[PWRAP_DEW_CRC_EN], 0x1))
+			return -EFAULT;
+
+		pwrap_writel(wrp, 0x1, PWRAP_CRC_EN);
+		pwrap_writel(wrp, 0x0, PWRAP_SIG_MODE);
+		pwrap_writel(wrp, wrp->slave->dew_regs[PWRAP_DEW_CRC_VAL],
+			     PWRAP_SIG_ADR);
+		pwrap_writel(wrp,
+			     wrp->master->arb_en_all, PWRAP_HIPRIO_ARB_EN);
+	}
 
 	if (wrp->master->type == PWRAP_MT8135)
 		pwrap_writel(wrp, 0x7, PWRAP_RRARB_EN);
@@ -1059,8 +1070,11 @@ static int pwrap_init(struct pmic_wrapper *wrp)
 	pwrap_writel(wrp, 0x1, PWRAP_WACS0_EN);
 	pwrap_writel(wrp, 0x1, PWRAP_WACS1_EN);
 	pwrap_writel(wrp, 0x1, PWRAP_WACS2_EN);
-	pwrap_writel(wrp, 0x5, PWRAP_STAUPD_PRD);
-	pwrap_writel(wrp, 0xff, PWRAP_STAUPD_GRPEN);
+
+	if (wrp->master->slv_program) {
+		pwrap_writel(wrp, 0x5, PWRAP_STAUPD_PRD);
+		pwrap_writel(wrp, 0xff, PWRAP_STAUPD_GRPEN);
+	}
 
 	if (wrp->master->init_soc_specific) {
 		ret = wrp->master->init_soc_specific(wrp);
@@ -1146,6 +1160,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,
+	.slv_program = 1,
 	.init_reg_clock = pwrap_mt2701_init_reg_clock,
 	.init_soc_specific = pwrap_mt2701_init_soc_specific,
 };
@@ -1158,6 +1173,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,
+	.slv_program = 1,
 	.init_reg_clock = pwrap_mt8135_init_reg_clock,
 	.init_soc_specific = pwrap_mt8135_init_soc_specific,
 };
@@ -1170,6 +1186,7 @@ static const struct pmic_wrapper_type pwrap_mt8173 = {
 	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
 	.wdt_src = PWRAP_WDT_SRC_MASK_NO_STAUPD,
 	.has_bridge = 0,
+	.slv_program = 1,
 	.init_reg_clock = pwrap_mt8173_init_reg_clock,
 	.init_soc_specific = pwrap_mt8173_init_soc_specific,
 };
-- 
2.7.4

[PATCH v4 5/7] soc: mediatek: pwrap: add MediaTek MT6380 as one slave of pwrap

[sean.wang]

From: Sean Wang <sean.wang@mediatek.com>

Add MediaTek MT6380 regulator becoming one of PMIC wrapper slave
and also add extra new regmap_config of 32-bit mode for MT6380
since old regmap_config of 16-bit mode can't be fit into the need.

Signed-off-by: Chenglin Xu <chenglin.xu@mediatek.com>
Signed-off-by: Chen Zhong <chen.zhong@mediatek.com>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 drivers/soc/mediatek/mtk-pmic-wrap.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index 9c6d855..5e3a41b 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -501,6 +501,7 @@ struct pmic_wrapper;
 struct pwrap_slv_type {
 	const u32 *dew_regs;
 	enum pmic_type type;
+	const struct regmap_config *regmap;
 	/* pwrap operations are highly associated with the PMIC types,
 	 * so the pointers added increases flexibility allowing determination
 	 * which type is used by the detection through device tree.
@@ -1109,7 +1110,7 @@ static irqreturn_t pwrap_interrupt(int irqno, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static const struct regmap_config pwrap_regmap_config = {
+static const struct regmap_config pwrap_regmap_config16 = {
 	.reg_bits = 16,
 	.val_bits = 16,
 	.reg_stride = 2,
@@ -1118,9 +1119,19 @@ static const struct regmap_config pwrap_regmap_config = {
 	.max_register = 0xffff,
 };
 
+static const struct regmap_config pwrap_regmap_config32 = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+	.reg_read = pwrap_regmap_read,
+	.reg_write = pwrap_regmap_write,
+	.max_register = 0xffff,
+};
+
 static const struct pwrap_slv_type pmic_mt6323 = {
 	.dew_regs = mt6323_regs,
 	.type = PMIC_MT6323,
+	.regmap = &pwrap_regmap_config16,
 	.pwrap_read = pwrap_read16,
 	.pwrap_write = pwrap_write16,
 };
@@ -1128,6 +1139,7 @@ static const struct pwrap_slv_type pmic_mt6323 = {
 static const struct pwrap_slv_type pmic_mt6380 = {
 	.dew_regs = NULL,
 	.type = PMIC_MT6380,
+	.regmap = &pwrap_regmap_config32,
 	.pwrap_read = pwrap_read32,
 	.pwrap_write = pwrap_write32,
 };
@@ -1135,6 +1147,7 @@ static const struct pwrap_slv_type pmic_mt6380 = {
 static const struct pwrap_slv_type pmic_mt6397 = {
 	.dew_regs = mt6397_regs,
 	.type = PMIC_MT6397,
+	.regmap = &pwrap_regmap_config16,
 	.pwrap_read = pwrap_read16,
 	.pwrap_write = pwrap_write16,
 };
@@ -1144,9 +1157,15 @@ static const struct of_device_id of_slave_match_tbl[] = {
 		.compatible = "mediatek,mt6323",
 		.data = &pmic_mt6323,
 	}, {
+		/* The MT6380 slave device is directly pointed to the regulator
+		 * device which is different from the cases MT6323 and MT6397
+		 * where they're one kind of MFDs.
+		 */
+		.compatible = "mediatek,mt6380-regulator",
+		.data = &pmic_mt6380,
+	}, {
 		.compatible = "mediatek,mt6397",
 		.data = &pmic_mt6397,
-	}, {
 		/* sentinel */
 	}
 };
@@ -1325,7 +1344,7 @@ static int pwrap_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_out2;
 
-	wrp->regmap = devm_regmap_init(wrp->dev, NULL, wrp, &pwrap_regmap_config);
+	wrp->regmap = devm_regmap_init(wrp->dev, NULL, wrp, wrp->slave->regmap);
 	if (IS_ERR(wrp->regmap)) {
 		ret = PTR_ERR(wrp->regmap);
 		goto err_out2;
-- 
2.7.4

[PATCH v4 6/7] soc: mediatek: pwrap: add support for MT7622 SoC

[sean.wang-NuS5LvNUpcJWk0Htik3J/w]

From: Chenglin Xu <chenglin.xu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>

Add the registers, callbacks and data structures required to make the
PMIC wrapper work on MT7622.

Signed-off-by: Chenglin Xu <chenglin.xu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
Signed-off-by: Chen Zhong <chen.zhong-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
Signed-off-by: Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 drivers/soc/mediatek/mtk-pmic-wrap.c | 180 +++++++++++++++++++++++++++++++++++
 1 file changed, 180 insertions(+)

diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index 5e3a41b..567d038 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -208,6 +208,36 @@ enum pwrap_regs {
 	PWRAP_ADC_RDATA_ADDR1,
 	PWRAP_ADC_RDATA_ADDR2,
 
+	/* MT7622 only regs */
+	PWRAP_EINT_STA0_ADR,
+	PWRAP_EINT_STA1_ADR,
+	PWRAP_STA,
+	PWRAP_CLR,
+	PWRAP_DVFS_ADR8,
+	PWRAP_DVFS_WDATA8,
+	PWRAP_DVFS_ADR9,
+	PWRAP_DVFS_WDATA9,
+	PWRAP_DVFS_ADR10,
+	PWRAP_DVFS_WDATA10,
+	PWRAP_DVFS_ADR11,
+	PWRAP_DVFS_WDATA11,
+	PWRAP_DVFS_ADR12,
+	PWRAP_DVFS_WDATA12,
+	PWRAP_DVFS_ADR13,
+	PWRAP_DVFS_WDATA13,
+	PWRAP_DVFS_ADR14,
+	PWRAP_DVFS_WDATA14,
+	PWRAP_DVFS_ADR15,
+	PWRAP_DVFS_WDATA15,
+	PWRAP_EXT_CK,
+	PWRAP_ADC_RDATA_ADDR,
+	PWRAP_GPS_STA,
+	PWRAP_SW_RST,
+	PWRAP_DVFS_STEP_CTRL0,
+	PWRAP_DVFS_STEP_CTRL1,
+	PWRAP_DVFS_STEP_CTRL2,
+	PWRAP_SPI2_CTRL,
+
 	/* MT8135 only regs */
 	PWRAP_CSHEXT,
 	PWRAP_EVENT_IN_EN,
@@ -330,6 +360,118 @@ static int mt2701_regs[] = {
 	[PWRAP_ADC_RDATA_ADDR2] =	0x154,
 };
 
+static int mt7622_regs[] = {
+	[PWRAP_MUX_SEL] =		0x0,
+	[PWRAP_WRAP_EN] =		0x4,
+	[PWRAP_DIO_EN] =		0x8,
+	[PWRAP_SIDLY] =			0xC,
+	[PWRAP_RDDMY] =			0x10,
+	[PWRAP_SI_CK_CON] =		0x14,
+	[PWRAP_CSHEXT_WRITE] =		0x18,
+	[PWRAP_CSHEXT_READ] =		0x1C,
+	[PWRAP_CSLEXT_START] =		0x20,
+	[PWRAP_CSLEXT_END] =		0x24,
+	[PWRAP_STAUPD_PRD] =		0x28,
+	[PWRAP_STAUPD_GRPEN] =		0x2C,
+	[PWRAP_EINT_STA0_ADR] =		0x30,
+	[PWRAP_EINT_STA1_ADR] =		0x34,
+	[PWRAP_STA] =			0x38,
+	[PWRAP_CLR] =			0x3C,
+	[PWRAP_STAUPD_MAN_TRIG] =	0x40,
+	[PWRAP_STAUPD_STA] =		0x44,
+	[PWRAP_WRAP_STA] =		0x48,
+	[PWRAP_HARB_INIT] =		0x4C,
+	[PWRAP_HARB_HPRIO] =		0x50,
+	[PWRAP_HIPRIO_ARB_EN] =		0x54,
+	[PWRAP_HARB_STA0] =		0x58,
+	[PWRAP_HARB_STA1] =		0x5C,
+	[PWRAP_MAN_EN] =		0x60,
+	[PWRAP_MAN_CMD] =		0x64,
+	[PWRAP_MAN_RDATA] =		0x68,
+	[PWRAP_MAN_VLDCLR] =		0x6C,
+	[PWRAP_WACS0_EN] =		0x70,
+	[PWRAP_INIT_DONE0] =		0x74,
+	[PWRAP_WACS0_CMD] =		0x78,
+	[PWRAP_WACS0_RDATA] =		0x7C,
+	[PWRAP_WACS0_VLDCLR] =		0x80,
+	[PWRAP_WACS1_EN] =		0x84,
+	[PWRAP_INIT_DONE1] =		0x88,
+	[PWRAP_WACS1_CMD] =		0x8C,
+	[PWRAP_WACS1_RDATA] =		0x90,
+	[PWRAP_WACS1_VLDCLR] =		0x94,
+	[PWRAP_WACS2_EN] =		0x98,
+	[PWRAP_INIT_DONE2] =		0x9C,
+	[PWRAP_WACS2_CMD] =		0xA0,
+	[PWRAP_WACS2_RDATA] =		0xA4,
+	[PWRAP_WACS2_VLDCLR] =		0xA8,
+	[PWRAP_INT_EN] =		0xAC,
+	[PWRAP_INT_FLG_RAW] =		0xB0,
+	[PWRAP_INT_FLG] =		0xB4,
+	[PWRAP_INT_CLR] =		0xB8,
+	[PWRAP_SIG_ADR] =		0xBC,
+	[PWRAP_SIG_MODE] =		0xC0,
+	[PWRAP_SIG_VALUE] =		0xC4,
+	[PWRAP_SIG_ERRVAL] =		0xC8,
+	[PWRAP_CRC_EN] =		0xCC,
+	[PWRAP_TIMER_EN] =		0xD0,
+	[PWRAP_TIMER_STA] =		0xD4,
+	[PWRAP_WDT_UNIT] =		0xD8,
+	[PWRAP_WDT_SRC_EN] =		0xDC,
+	[PWRAP_WDT_FLG] =		0xE0,
+	[PWRAP_DEBUG_INT_SEL] =		0xE4,
+	[PWRAP_DVFS_ADR0] =		0xE8,
+	[PWRAP_DVFS_WDATA0] =		0xEC,
+	[PWRAP_DVFS_ADR1] =		0xF0,
+	[PWRAP_DVFS_WDATA1] =		0xF4,
+	[PWRAP_DVFS_ADR2] =		0xF8,
+	[PWRAP_DVFS_WDATA2] =		0xFC,
+	[PWRAP_DVFS_ADR3] =		0x100,
+	[PWRAP_DVFS_WDATA3] =		0x104,
+	[PWRAP_DVFS_ADR4] =		0x108,
+	[PWRAP_DVFS_WDATA4] =		0x10C,
+	[PWRAP_DVFS_ADR5] =		0x110,
+	[PWRAP_DVFS_WDATA5] =		0x114,
+	[PWRAP_DVFS_ADR6] =		0x118,
+	[PWRAP_DVFS_WDATA6] =		0x11C,
+	[PWRAP_DVFS_ADR7] =		0x120,
+	[PWRAP_DVFS_WDATA7] =		0x124,
+	[PWRAP_DVFS_ADR8] =		0x128,
+	[PWRAP_DVFS_WDATA8] =		0x12C,
+	[PWRAP_DVFS_ADR9] =		0x130,
+	[PWRAP_DVFS_WDATA9] =		0x134,
+	[PWRAP_DVFS_ADR10] =		0x138,
+	[PWRAP_DVFS_WDATA10] =		0x13C,
+	[PWRAP_DVFS_ADR11] =		0x140,
+	[PWRAP_DVFS_WDATA11] =		0x144,
+	[PWRAP_DVFS_ADR12] =		0x148,
+	[PWRAP_DVFS_WDATA12] =		0x14C,
+	[PWRAP_DVFS_ADR13] =		0x150,
+	[PWRAP_DVFS_WDATA13] =		0x154,
+	[PWRAP_DVFS_ADR14] =		0x158,
+	[PWRAP_DVFS_WDATA14] =		0x15C,
+	[PWRAP_DVFS_ADR15] =		0x160,
+	[PWRAP_DVFS_WDATA15] =		0x164,
+	[PWRAP_SPMINF_STA] =		0x168,
+	[PWRAP_CIPHER_KEY_SEL] =	0x16C,
+	[PWRAP_CIPHER_IV_SEL] =		0x170,
+	[PWRAP_CIPHER_EN] =		0x174,
+	[PWRAP_CIPHER_RDY] =		0x178,
+	[PWRAP_CIPHER_MODE] =		0x17C,
+	[PWRAP_CIPHER_SWRST] =		0x180,
+	[PWRAP_DCM_EN] =		0x184,
+	[PWRAP_DCM_DBC_PRD] =		0x188,
+	[PWRAP_EXT_CK] =		0x18C,
+	[PWRAP_ADC_CMD_ADDR] =		0x190,
+	[PWRAP_PWRAP_ADC_CMD] =		0x194,
+	[PWRAP_ADC_RDATA_ADDR] =	0x198,
+	[PWRAP_GPS_STA] =		0x19C,
+	[PWRAP_SW_RST] =		0x1A0,
+	[PWRAP_DVFS_STEP_CTRL0] =	0x238,
+	[PWRAP_DVFS_STEP_CTRL1] =	0x23C,
+	[PWRAP_DVFS_STEP_CTRL2] =	0x240,
+	[PWRAP_SPI2_CTRL] =		0x244,
+};
+
 static int mt8173_regs[] = {
 	[PWRAP_MUX_SEL] =		0x0,
 	[PWRAP_WRAP_EN] =		0x4,
@@ -493,6 +635,7 @@ enum pmic_type {
 
 enum pwrap_type {
 	PWRAP_MT2701,
+	PWRAP_MT7622,
 	PWRAP_MT8135,
 	PWRAP_MT8173,
 };
@@ -829,6 +972,16 @@ static int pwrap_mt2701_init_reg_clock(struct pmic_wrapper *wrp)
 	return 0;
 }
 
+static int pwrap_mt7622_init_reg_clock(struct pmic_wrapper *wrp)
+{
+	pwrap_writel(wrp, 0x0, PWRAP_CSHEXT_WRITE);
+	pwrap_writel(wrp, 0x4, PWRAP_CSHEXT_READ);
+	pwrap_writel(wrp, 0x2, PWRAP_CSLEXT_START);
+	pwrap_writel(wrp, 0x2, PWRAP_CSLEXT_END);
+
+	return 0;
+}
+
 static bool pwrap_is_cipher_ready(struct pmic_wrapper *wrp)
 {
 	return pwrap_readl(wrp, PWRAP_CIPHER_RDY) & 1;
@@ -866,6 +1019,9 @@ static int pwrap_init_cipher(struct pmic_wrapper *wrp)
 	case PWRAP_MT8173:
 		pwrap_writel(wrp, 1, PWRAP_CIPHER_EN);
 		break;
+	case PWRAP_MT7622:
+		pwrap_writel(wrp, 0, PWRAP_CIPHER_EN);
+		break;
 	}
 
 	/* Config cipher mode @PMIC */
@@ -985,6 +1141,15 @@ static int pwrap_mt2701_init_soc_specific(struct pmic_wrapper *wrp)
 	return 0;
 }
 
+static int pwrap_mt7622_init_soc_specific(struct pmic_wrapper *wrp)
+{
+	pwrap_writel(wrp, 0, PWRAP_STAUPD_PRD);
+	/*enable 2wire SPI master*/
+	pwrap_writel(wrp, 0x8000000, PWRAP_SPI2_CTRL);
+
+	return 0;
+}
+
 static int pwrap_init(struct pmic_wrapper *wrp)
 {
 	int ret;
@@ -1184,6 +1349,18 @@ static const struct pmic_wrapper_type pwrap_mt2701 = {
 	.init_soc_specific = pwrap_mt2701_init_soc_specific,
 };
 
+static struct pmic_wrapper_type pwrap_mt7622 = {
+	.regs = mt7622_regs,
+	.type = PWRAP_MT7622,
+	.arb_en_all = 0xff,
+	.int_en_all = ~(u32)BIT(31),
+	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
+	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
+	.has_bridge = 0,
+	.init_reg_clock = pwrap_mt7622_init_reg_clock,
+	.init_soc_specific = pwrap_mt7622_init_soc_specific,
+};
+
 static const struct pmic_wrapper_type pwrap_mt8135 = {
 	.regs = mt8135_regs,
 	.type = PWRAP_MT8135,
@@ -1215,6 +1392,9 @@ static const struct of_device_id of_pwrap_match_tbl[] = {
 		.compatible = "mediatek,mt2701-pwrap",
 		.data = &pwrap_mt2701,
 	}, {
+		.compatible = "mediatek,mt7622-pwrap",
+		.data = &pwrap_mt7622,
+	}, {
 		.compatible = "mediatek,mt8135-pwrap",
 		.data = &pwrap_mt8135,
 	}, {
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 7/7] soc: mediatek: pwrap: fixup warnings from coding style

[sean.wang]

From: Sean Wang <sean.wang@mediatek.com>

fixup those warnings such as lines over 80 words and parenthesis
alignment which would be complained by checkpatch.pl.

Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 drivers/soc/mediatek/mtk-pmic-wrap.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index 567d038..2d13cf6 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -1057,7 +1057,8 @@ static int pwrap_init_cipher(struct pmic_wrapper *wrp)
 	/* wait for cipher data ready@PMIC */
 	ret = pwrap_wait_for_state(wrp, pwrap_is_pmic_cipher_ready);
 	if (ret) {
-		dev_err(wrp->dev, "timeout waiting for cipher data ready@PMIC\n");
+		dev_err(wrp->dev,
+			"timeout waiting for cipher data ready@PMIC\n");
 		return ret;
 	}
 
@@ -1458,23 +1459,27 @@ static int pwrap_probe(struct platform_device *pdev)
 		if (IS_ERR(wrp->bridge_base))
 			return PTR_ERR(wrp->bridge_base);
 
-		wrp->rstc_bridge = devm_reset_control_get(wrp->dev, "pwrap-bridge");
+		wrp->rstc_bridge = devm_reset_control_get(wrp->dev,
+							  "pwrap-bridge");
 		if (IS_ERR(wrp->rstc_bridge)) {
 			ret = PTR_ERR(wrp->rstc_bridge);
-			dev_dbg(wrp->dev, "cannot get pwrap-bridge reset: %d\n", ret);
+			dev_dbg(wrp->dev,
+				"cannot get pwrap-bridge reset: %d\n", ret);
 			return ret;
 		}
 	}
 
 	wrp->clk_spi = devm_clk_get(wrp->dev, "spi");
 	if (IS_ERR(wrp->clk_spi)) {
-		dev_dbg(wrp->dev, "failed to get clock: %ld\n", PTR_ERR(wrp->clk_spi));
+		dev_dbg(wrp->dev, "failed to get clock: %ld\n",
+			PTR_ERR(wrp->clk_spi));
 		return PTR_ERR(wrp->clk_spi);
 	}
 
 	wrp->clk_wrap = devm_clk_get(wrp->dev, "wrap");
 	if (IS_ERR(wrp->clk_wrap)) {
-		dev_dbg(wrp->dev, "failed to get clock: %ld\n", PTR_ERR(wrp->clk_wrap));
+		dev_dbg(wrp->dev, "failed to get clock: %ld\n",
+			PTR_ERR(wrp->clk_wrap));
 		return PTR_ERR(wrp->clk_wrap);
 	}
 
@@ -1519,8 +1524,9 @@ static int pwrap_probe(struct platform_device *pdev)
 	pwrap_writel(wrp, wrp->master->int_en_all, PWRAP_INT_EN);
 
 	irq = platform_get_irq(pdev, 0);
-	ret = devm_request_irq(wrp->dev, irq, pwrap_interrupt, IRQF_TRIGGER_HIGH,
-			"mt-pmic-pwrap", wrp);
+	ret = devm_request_irq(wrp->dev, irq, pwrap_interrupt,
+			       IRQF_TRIGGER_HIGH,
+			       "mt-pmic-pwrap", wrp);
 	if (ret)
 		goto err_out2;
 
-- 
2.7.4

Re: [PATCH v4 4/7] soc: mediatek: pwrap: update pwrap_init without slave programming

[Matthias Brugger]

On 09/21/2017 10:26 AM, sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org wrote:
> From: Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> 
> pwrap initialization is highly associated with the base SoC, so
> update here for allowing pwrap_init without slave program which would be
> used to those PMICs without extra encryption on bus such as MT6380.
> 
> Signed-off-by: Chenglin Xu <chenglin.xu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Chen Zhong <chen.zhong-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
>   drivers/soc/mediatek/mtk-pmic-wrap.c | 91 +++++++++++++++++++++---------------
>   1 file changed, 54 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index 27d7ccc..9c6d855 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -531,6 +531,7 @@ struct pmic_wrapper_type {
>   	u32 spi_w;
>   	u32 wdt_src;
>   	int has_bridge:1;
> +	int slv_program:1;
>   	int (*init_reg_clock)(struct pmic_wrapper *wrp);
>   	int (*init_soc_specific)(struct pmic_wrapper *wrp);
>   };
> @@ -999,9 +1000,12 @@ static int pwrap_init(struct pmic_wrapper *wrp)
>   	}
>   
>   	/* Reset SPI slave */
> -	ret = pwrap_reset_spislave(wrp);
> -	if (ret)
> -		return ret;
> +
> +	if (wrp->master->slv_program) {
> +		ret = pwrap_reset_spislave(wrp);
> +		if (ret)
> +			return ret;
> +	}
>   
>   	pwrap_writel(wrp, 1, PWRAP_WRAP_EN);
>   
> @@ -1013,45 +1017,52 @@ static int pwrap_init(struct pmic_wrapper *wrp)
>   	if (ret)
>   		return ret;
>   
> -	/* Setup serial input delay */
> -	ret = pwrap_init_sidly(wrp);
> -	if (ret)
> -		return ret;
> +	if (wrp->master->slv_program) {

This if branch is really long and complex enough to put it into function apart.

Thanks,
Matthias

PD please take into account the comments I made on v3 of the series.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 4/7] soc: mediatek: pwrap: update pwrap_init without slave programming

[Sean Wang]

On Tue, 2017-10-10 at 20:00 +0200, Matthias Brugger wrote:
> 
> On 09/21/2017 10:26 AM, sean.wang@mediatek.com wrote:
> > From: Sean Wang <sean.wang@mediatek.com>
> > 
> > pwrap initialization is highly associated with the base SoC, so
> > update here for allowing pwrap_init without slave program which would be
> > used to those PMICs without extra encryption on bus such as MT6380.
> > 
> > Signed-off-by: Chenglin Xu <chenglin.xu@mediatek.com>
> > Signed-off-by: Chen Zhong <chen.zhong@mediatek.com>
> > Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> > ---
> >   drivers/soc/mediatek/mtk-pmic-wrap.c | 91 +++++++++++++++++++++---------------
> >   1 file changed, 54 insertions(+), 37 deletions(-)
> > 
> > diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> > index 27d7ccc..9c6d855 100644
> > --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> > +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> > @@ -531,6 +531,7 @@ struct pmic_wrapper_type {
> >   	u32 spi_w;
> >   	u32 wdt_src;
> >   	int has_bridge:1;
> > +	int slv_program:1;
> >   	int (*init_reg_clock)(struct pmic_wrapper *wrp);
> >   	int (*init_soc_specific)(struct pmic_wrapper *wrp);
> >   };
> > @@ -999,9 +1000,12 @@ static int pwrap_init(struct pmic_wrapper *wrp)
> >   	}
> >   
> >   	/* Reset SPI slave */
> > -	ret = pwrap_reset_spislave(wrp);
> > -	if (ret)
> > -		return ret;
> > +
> > +	if (wrp->master->slv_program) {
> > +		ret = pwrap_reset_spislave(wrp);
> > +		if (ret)
> > +			return ret;
> > +	}
> >   
> >   	pwrap_writel(wrp, 1, PWRAP_WRAP_EN);
> >   
> > @@ -1013,45 +1017,52 @@ static int pwrap_init(struct pmic_wrapper *wrp)
> >   	if (ret)
> >   		return ret;
> >   
> > -	/* Setup serial input delay */
> > -	ret = pwrap_init_sidly(wrp);
> > -	if (ret)
> > -		return ret;
> > +	if (wrp->master->slv_program) {
> 
> This if branch is really long and complex enough to put it into function apart.
> 
> Thanks,
> Matthias
> 
> PD please take into account the comments I made on v3 of the series.
> 

I'll try to breakdown the long logic into the short one and use a flag
indicating the slave capability decides whether the functions is
required being enabled for the slave instead of slv_program which is
less meaningful. In this way, pmic_init will be more extensible when
more different SoCs and target slaves with various flavors into the
driver. And also take into accounts those suggestions you made in v3 in
the next version.

	Sean

Re: [PATCH v4 4/7] soc: mediatek: pwrap: update pwrap_init without slave programming

[Matthias Brugger]

On 10/13/2017 11:41 AM, Sean Wang wrote:
> On Tue, 2017-10-10 at 20:00 +0200, Matthias Brugger wrote:
>>
>> On 09/21/2017 10:26 AM, sean.wang@mediatek.com wrote:
>>> From: Sean Wang <sean.wang@mediatek.com>
>>>
>>> pwrap initialization is highly associated with the base SoC, so
>>> update here for allowing pwrap_init without slave program which would be
>>> used to those PMICs without extra encryption on bus such as MT6380.
>>>
>>> Signed-off-by: Chenglin Xu <chenglin.xu@mediatek.com>
>>> Signed-off-by: Chen Zhong <chen.zhong@mediatek.com>
>>> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
>>> ---
>>>    drivers/soc/mediatek/mtk-pmic-wrap.c | 91 +++++++++++++++++++++---------------
>>>    1 file changed, 54 insertions(+), 37 deletions(-)
>>>
>>> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
>>> index 27d7ccc..9c6d855 100644
>>> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
>>> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
>>> @@ -531,6 +531,7 @@ struct pmic_wrapper_type {
>>>    	u32 spi_w;
>>>    	u32 wdt_src;
>>>    	int has_bridge:1;
>>> +	int slv_program:1;
>>>    	int (*init_reg_clock)(struct pmic_wrapper *wrp);
>>>    	int (*init_soc_specific)(struct pmic_wrapper *wrp);
>>>    };
>>> @@ -999,9 +1000,12 @@ static int pwrap_init(struct pmic_wrapper *wrp)
>>>    	}
>>>    
>>>    	/* Reset SPI slave */
>>> -	ret = pwrap_reset_spislave(wrp);
>>> -	if (ret)
>>> -		return ret;
>>> +
>>> +	if (wrp->master->slv_program) {
>>> +		ret = pwrap_reset_spislave(wrp);
>>> +		if (ret)
>>> +			return ret;
>>> +	}
>>>    
>>>    	pwrap_writel(wrp, 1, PWRAP_WRAP_EN);
>>>    
>>> @@ -1013,45 +1017,52 @@ static int pwrap_init(struct pmic_wrapper *wrp)
>>>    	if (ret)
>>>    		return ret;
>>>    
>>> -	/* Setup serial input delay */
>>> -	ret = pwrap_init_sidly(wrp);
>>> -	if (ret)
>>> -		return ret;
>>> +	if (wrp->master->slv_program) {
>>
>> This if branch is really long and complex enough to put it into function apart.
>>
>> Thanks,
>> Matthias
>>
>> PD please take into account the comments I made on v3 of the series.
>>
> 
> I'll try to breakdown the long logic into the short one and use a flag
> indicating the slave capability decides whether the functions is
> required being enabled for the slave instead of slv_program which is
> less meaningful. In this way, pmic_init will be more extensible when
> more different SoCs and target slaves with various flavors into the
> driver. And also take into accounts those suggestions you made in v3 in
> the next version.
> 
> 	Sean
> 

I totally agree, but I wanted to underline that right now the if branch under
"if (wrp->master->slv_program)" is around 30 lines, so I think it would be a 
good candidate to put it into it's own function. For example: 
pwrap_init_encryption()
As from what I understand from the commit log, slv_program in the end enables 
encryption of the communication, right?

Regards,
Matthias

Re: [PATCH v4 4/7] soc: mediatek: pwrap: update pwrap_init without slave programming

[Sean Wang]

On Fri, 2017-10-13 at 16:07 +0200, Matthias Brugger wrote:
> 
> On 10/13/2017 11:41 AM, Sean Wang wrote:
> > On Tue, 2017-10-10 at 20:00 +0200, Matthias Brugger wrote:
> >>
> >> On 09/21/2017 10:26 AM, sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org wrote:
> >>> From: Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> >>>
> >>> pwrap initialization is highly associated with the base SoC, so
> >>> update here for allowing pwrap_init without slave program which would be
> >>> used to those PMICs without extra encryption on bus such as MT6380.
> >>>
> >>> Signed-off-by: Chenglin Xu <chenglin.xu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> >>> Signed-off-by: Chen Zhong <chen.zhong-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> >>> Signed-off-by: Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> >>> ---
> >>>    drivers/soc/mediatek/mtk-pmic-wrap.c | 91 +++++++++++++++++++++---------------
> >>>    1 file changed, 54 insertions(+), 37 deletions(-)
> >>>
> >>> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> >>> index 27d7ccc..9c6d855 100644
> >>> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> >>> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> >>> @@ -531,6 +531,7 @@ struct pmic_wrapper_type {
> >>>    	u32 spi_w;
> >>>    	u32 wdt_src;
> >>>    	int has_bridge:1;
> >>> +	int slv_program:1;
> >>>    	int (*init_reg_clock)(struct pmic_wrapper *wrp);
> >>>    	int (*init_soc_specific)(struct pmic_wrapper *wrp);
> >>>    };
> >>> @@ -999,9 +1000,12 @@ static int pwrap_init(struct pmic_wrapper *wrp)
> >>>    	}
> >>>    
> >>>    	/* Reset SPI slave */
> >>> -	ret = pwrap_reset_spislave(wrp);
> >>> -	if (ret)
> >>> -		return ret;
> >>> +
> >>> +	if (wrp->master->slv_program) {
> >>> +		ret = pwrap_reset_spislave(wrp);
> >>> +		if (ret)
> >>> +			return ret;
> >>> +	}
> >>>    
> >>>    	pwrap_writel(wrp, 1, PWRAP_WRAP_EN);
> >>>    
> >>> @@ -1013,45 +1017,52 @@ static int pwrap_init(struct pmic_wrapper *wrp)
> >>>    	if (ret)
> >>>    		return ret;
> >>>    
> >>> -	/* Setup serial input delay */
> >>> -	ret = pwrap_init_sidly(wrp);
> >>> -	if (ret)
> >>> -		return ret;
> >>> +	if (wrp->master->slv_program) {
> >>
> >> This if branch is really long and complex enough to put it into function apart.
> >>
> >> Thanks,
> >> Matthias
> >>
> >> PD please take into account the comments I made on v3 of the series.
> >>
> > 
> > I'll try to breakdown the long logic into the short one and use a flag
> > indicating the slave capability decides whether the functions is
> > required being enabled for the slave instead of slv_program which is
> > less meaningful. In this way, pmic_init will be more extensible when
> > more different SoCs and target slaves with various flavors into the
> > driver. And also take into accounts those suggestions you made in v3 in
> > the next version.
> > 
> > 	Sean
> > 
> 
> I totally agree, but I wanted to underline that right now the if branch under
> "if (wrp->master->slv_program)" is around 30 lines, so I think it would be a 
> good candidate to put it into it's own function. For example: 
> pwrap_init_encryption()

understood. I'll try to turn the code block into a function.

> As from what I understand from the commit log, slv_program in the end enables 
> encryption of the communication, right?
> 

yes. slv_program in the end will enable the security feature over the
bus such as the encryption and the signature for the data integrity when
the pwrap slave can support it.


> Regards,
> Matthias


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html