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

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

[sean.wang]

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

By default, the relevant board with MediaTek MT7622 SoC is equipped with
MediaTek MT6380 PMIC. So the series enables related driver allows the
MT6380 can be accessed and controlled through MediaTek PMIC wrapper by
those function drivers on MT7622 SoC.

Patch 1-2: Update dt-binding document for PMIC Wrapper and MT6380 regulator.
Patch 3:   Add a new driver for MT6380 regulator.
Patch 4-7: Update operations in PMIC Wrapper in order to adapt into MT6380.
Patch 8:   Add support of MT7622 platform for PMIC Wrapper
Patch 9:   Fix warning complained from checkpatch.pl.

Chenglin Xu (2):
  regulator: mt6380: Add support for MT6380
  soc: mediatek: pwrap: add support for MT7622 SoC

Sean Wang (7):
  dt-bindings: arm: mediatek: add MT7622 string to the PMIC wrapper doc
  dt-bindings: regulator: Add document for MediaTek MT6380 regulator
  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: add pwrap_init pointer decided by the actual
    PMIC
  soc: mediatek: pwrap: add MediaTek MT6380 as one slave of pwrap
  soc: mediatek: pwrap: fixup warnings from coding style

 .../bindings/regulator/mt6380-regulator.txt        |  90 +++++
 .../devicetree/bindings/soc/mediatek/pwrap.txt     |   4 +
 drivers/regulator/Kconfig                          |   9 +
 drivers/regulator/Makefile                         |   1 +
 drivers/regulator/mt6380-regulator.c               | 395 +++++++++++++++++++++
 drivers/soc/mediatek/mtk-pmic-wrap.c               | 373 +++++++++++++++++--
 include/linux/regulator/mt6380-regulator.h         |  32 ++
 7 files changed, 883 insertions(+), 21 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/mt6380-regulator.txt
 create mode 100644 drivers/regulator/mt6380-regulator.c
 create mode 100644 include/linux/regulator/mt6380-regulator.h

-- 
1.9.1

[PATCH 1/9] 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>
---
 Documentation/devicetree/bindings/soc/mediatek/pwrap.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/mediatek/pwrap.txt b/Documentation/devicetree/bindings/soc/mediatek/pwrap.txt
index 107700d..2d6d4a9 100644
--- a/Documentation/devicetree/bindings/soc/mediatek/pwrap.txt
+++ b/Documentation/devicetree/bindings/soc/mediatek/pwrap.txt
@@ -35,6 +35,10 @@ Required properties in pwrap device node.
   "wrap": Main module clock
 - clocks: Must contain an entry for each entry in clock-names.
 
+Required properties in pwrap device node
+- compatible:
+	"mediatek,mt7622-pwrap" for MT7622 SoCs
+
 Optional properities:
 - pmic: Mediatek PMIC MFD is the child device of pwrap
   See the following for child node definitions:
-- 
1.9.1

[PATCH 2/9] dt-bindings: regulator: Add document for MediaTek MT6380 regulator

[sean.wang]

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

add dt-binding document for MediaTek MT6380 PMIC

Signed-off-by: Chenglin Xu <chenglin.xu@mediatek.com>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 .../bindings/regulator/mt6380-regulator.txt        | 90 ++++++++++++++++++++++
 1 file changed, 90 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/mt6380-regulator.txt

diff --git a/Documentation/devicetree/bindings/regulator/mt6380-regulator.txt b/Documentation/devicetree/bindings/regulator/mt6380-regulator.txt
new file mode 100644
index 0000000..9affeab
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/mt6380-regulator.txt
@@ -0,0 +1,90 @@
+Mediatek MT6380 Regulator Driver
+
+All voltage regulators are defined as subnodes of the regulators node. A list
+of regulators provided by this controller are defined as subnodes of the
+PMIC's node. Each regulator is named according to its regulator type,
+buck_<name> and ldo_<name>. The definition for each of these nodes is defined
+using the standard binding for regulators at
+Documentation/devicetree/bindings/regulator/regulator.txt.
+
+The valid names for regulators are::
+BUCK:
+  buck_vcore1, buck_vcore, buck_vrf
+LDO:
+  ldo_vmldo ,ldo_valdo , ldo_vphyldo, ldo_vddrldo, ldo_vtldo
+
+Example:
+
+	mt6380regulator: mt6380regulator {
+		compatible = "mediatek,mt6380-regulator";
+
+		mt6380_vcpu_reg: buck_vcore1 {
+			regulator-name = "vcpu";
+			regulator-min-microvolt = < 600000>;
+			regulator-max-microvolt = <1393750>;
+			regulator-ramp-delay = <6250>;
+			regulator-always-on;
+			regulator-boot-on;
+		};
+
+		mt6380_vcore_reg: buck_vcore {
+			regulator-name = "vcore";
+			regulator-min-microvolt = <600000>;
+			regulator-max-microvolt = <1393750>;
+			regulator-ramp-delay = <6250>;
+		};
+
+		mt6380_vrf_reg: buck_vrf {
+			regulator-name = "vrf";
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1575000>;
+			regulator-ramp-delay = <0>;
+			regulator-always-on;
+			regulator-boot-on;
+		};
+
+		mt6380_vm_reg: ldo_vmldo {
+			regulator-name = "vmldo";
+			regulator-min-microvolt = <1050000>;
+			regulator-max-microvolt = <1400000>;
+			regulator-ramp-delay = <0>;
+			regulator-always-on;
+			regulator-boot-on;
+		};
+
+		mt6380_va_reg: ldo_valdo {
+			regulator-name = "valdo";
+			regulator-min-microvolt = <2200000>;
+			regulator-max-microvolt = <3300000>;
+			regulator-ramp-delay = <0>;
+			regulator-always-on;
+			regulator-boot-on;
+		};
+
+		mt6380_vphy_reg: ldo_vphyldo {
+			regulator-name = "vphyldo";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			regulator-ramp-delay = <0>;
+			regulator-always-on;
+			regulator-boot-on;
+		};
+
+		mt6380_vddr_reg: ldo_vddrldo {
+			regulator-name = "vddr";
+			regulator-min-microvolt = <1240000>;
+			regulator-max-microvolt = <1840000>;
+			regulator-ramp-delay = <0>;
+			regulator-always-on;
+			regulator-boot-on;
+		};
+
+		mt6380_vt_reg: ldo_vtldo {
+			regulator-name = "vadc18";
+			regulator-min-microvolt = <2200000>;
+			regulator-max-microvolt = <3300000>;
+			regulator-ramp-delay = <0>;
+			regulator-always-on;
+			regulator-boot-on;
+		};
+	};
-- 
1.9.1

[PATCH 3/9] regulator: mt6380: Add support for MT6380

[sean.wang]

From: Chenglin Xu <chenglin.xu@mediatek.com>

The MT6380 is a regulator found those boards with MediaTek MT7622 SoC
It is connected as a slave to the SoC using MediaTek PMIC wrapper which
is the common interface connecting with Mediatek made various PMICs.

Signed-off-by: Chenglin Xu <chenglin.xu@mediatek.com>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 drivers/regulator/Kconfig                  |   9 +
 drivers/regulator/Makefile                 |   1 +
 drivers/regulator/mt6380-regulator.c       | 395 +++++++++++++++++++++++++++++
 include/linux/regulator/mt6380-regulator.h |  32 +++
 4 files changed, 437 insertions(+)
 create mode 100644 drivers/regulator/mt6380-regulator.c
 create mode 100644 include/linux/regulator/mt6380-regulator.h

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 48db87d..c46ef9c 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -541,6 +541,15 @@ config REGULATOR_MT6323
 	  This driver supports the control of different power rails of device
 	  through regulator interface.
 
+config REGULATOR_MT6380
+	tristate "MediaTek MT6380 PMIC"
+	depends on MTK_PMIC_WRAP
+	help
+	  Say y here to select this option to enable the power regulator of
+	  MediaTek MT6380 PMIC.
+	  This driver supports the control of different power rails of device
+	  through regulator interface.
+
 config REGULATOR_MT6397
 	tristate "MediaTek MT6397 PMIC"
 	depends on MFD_MT6397
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index dc3503f..5148583 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -70,6 +70,7 @@ obj-$(CONFIG_REGULATOR_MC13892) += mc13892-regulator.o
 obj-$(CONFIG_REGULATOR_MC13XXX_CORE) +=  mc13xxx-regulator-core.o
 obj-$(CONFIG_REGULATOR_MT6311) += mt6311-regulator.o
 obj-$(CONFIG_REGULATOR_MT6323)	+= mt6323-regulator.o
+obj-$(CONFIG_REGULATOR_MT6380)	+= mt6380-regulator.o
 obj-$(CONFIG_REGULATOR_MT6397)	+= mt6397-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_RPM) += qcom_rpm-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_SMD_RPM) += qcom_smd-regulator.o
diff --git a/drivers/regulator/mt6380-regulator.c b/drivers/regulator/mt6380-regulator.c
new file mode 100644
index 0000000..7df1a4e
--- /dev/null
+++ b/drivers/regulator/mt6380-regulator.c
@@ -0,0 +1,395 @@
+/*
+ * Copyright (c) 2017 MediaTek Inc.
+ * Author: Chenglin Xu <chenglin.xu@mediatek.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/mt6380-regulator.h>
+#include <linux/regulator/of_regulator.h>
+
+/* PMIC Registers */
+#define MT6380_ALDO_CON_0                         0x0000
+#define MT6380_BTLDO_CON_0                        0x0004
+#define MT6380_COMP_CON_0                         0x0008
+#define MT6380_CPUBUCK_CON_0                      0x000C
+#define MT6380_CPUBUCK_CON_1                      0x0010
+#define MT6380_CPUBUCK_CON_2                      0x0014
+#define MT6380_DDRLDO_CON_0                       0x0018
+#define MT6380_MLDO_CON_0                         0x001C
+#define MT6380_PALDO_CON_0                        0x0020
+#define MT6380_PHYLDO_CON_0                       0x0024
+#define MT6380_SIDO_CON_0                         0x0028
+#define MT6380_SIDO_CON_1                         0x002C
+#define MT6380_SIDO_CON_2                         0x0030
+#define MT6380_SLDO_CON_0                         0x0034
+#define MT6380_TLDO_CON_0                         0x0038
+#define MT6380_STARTUP_CON_0                      0x003C
+#define MT6380_STARTUP_CON_1                      0x0040
+#define MT6380_SMPS_TOP_CON_0                     0x0044
+#define MT6380_SMPS_TOP_CON_1                     0x0048
+#define MT6380_ANA_CTRL_0                         0x0050
+#define MT6380_ANA_CTRL_1                         0x0054
+#define MT6380_ANA_CTRL_2                         0x0058
+#define MT6380_ANA_CTRL_3                         0x005C
+#define MT6380_ANA_CTRL_4                         0x0060
+#define MT6380_SPK_CON9                           0x0064
+#define MT6380_SPK_CON11                          0x0068
+#define MT6380_SPK_CON12                          0x006A
+#define MT6380_CLK_CTRL                           0x0070
+#define MT6380_PINMUX_CTRL                        0x0074
+#define MT6380_IO_CTRL                            0x0078
+#define MT6380_SLP_MODE_CTRL_0                    0x007C
+#define MT6380_SLP_MODE_CTRL_1                    0x0080
+#define MT6380_SLP_MODE_CTRL_2                    0x0084
+#define MT6380_SLP_MODE_CTRL_3                    0x0088
+#define MT6380_SLP_MODE_CTRL_4                    0x008C
+#define MT6380_SLP_MODE_CTRL_5                    0x0090
+#define MT6380_SLP_MODE_CTRL_6                    0x0094
+#define MT6380_SLP_MODE_CTRL_7                    0x0098
+#define MT6380_SLP_MODE_CTRL_8                    0x009C
+#define MT6380_FCAL_CTRL_0                        0x00A0
+#define MT6380_FCAL_CTRL_1                        0x00A4
+#define MT6380_LDO_CTRL_0                         0x00A8
+#define MT6380_LDO_CTRL_1                         0x00AC
+#define MT6380_LDO_CTRL_2                         0x00B0
+#define MT6380_LDO_CTRL_3                         0x00B4
+#define MT6380_LDO_CTRL_4                         0x00B8
+#define MT6380_DEBUG_CTRL_0                       0x00BC
+#define MT6380_EFU_CTRL_0                         0x0200
+#define MT6380_EFU_CTRL_1                         0x0201
+#define MT6380_EFU_CTRL_2                         0x0202
+#define MT6380_EFU_CTRL_3                         0x0203
+#define MT6380_EFU_CTRL_4                         0x0204
+#define MT6380_EFU_CTRL_5                         0x0205
+#define MT6380_EFU_CTRL_6                         0x0206
+#define MT6380_EFU_CTRL_7                         0x0207
+#define MT6380_EFU_CTRL_8                         0x0208
+
+#define MT6380_REGULATOR_MODE_NORMAL	0
+#define MT6380_REGULATOR_MODE_LP	1
+#define MT6380_REGULATOR_MODE_FORCE_PWM	1
+
+/*
+ * mt6380 regulators' information
+ *
+ * @desc: standard fields of regulator description.
+ * @qi: Mask for query enable signal status of regulators
+ * @vselon_reg: Register sections for hardware control mode of bucks
+ * @modeset_reg: Register for controlling the buck/LDO control mode
+ * @modeset_mask: Mask for controlling the buck/LDO control mode
+ */
+struct mt6380_regulator_info {
+	struct regulator_desc desc;
+	u32 qi;
+	u32 vselon_reg;
+	u32 modeset_reg;
+	u32 modeset_mask;
+};
+
+#define MT6380_BUCK(match, vreg, min, max, step, volt_ranges, enreg,	\
+		vosel, vosel_mask, enbit, voselon, _modeset_reg, \
+		_modeset_mask)	\
+[MT6380_ID_##vreg] = {							\
+	.desc = {							\
+		.name = #vreg,						\
+		.of_match = of_match_ptr(match),			\
+		.ops = &mt6380_volt_range_ops,				\
+		.type = REGULATOR_VOLTAGE,				\
+		.id = MT6380_ID_##vreg,					\
+		.owner = THIS_MODULE,					\
+		.n_voltages = ((max) - (min)) / (step) + 1,		\
+		.linear_ranges = volt_ranges,				\
+		.n_linear_ranges = ARRAY_SIZE(volt_ranges),		\
+		.vsel_reg = vosel,					\
+		.vsel_mask = vosel_mask,				\
+		.enable_reg = enreg,					\
+		.enable_mask = BIT(enbit),				\
+	},								\
+	.vselon_reg = voselon,						\
+	.modeset_reg = _modeset_reg,					\
+	.modeset_mask = _modeset_mask,					\
+}
+
+#define MT6380_LDO(match, vreg, ldo_volt_table, enreg, enbit, vosel,	\
+		vosel_mask, _modeset_reg, _modeset_mask)		\
+[MT6380_ID_##vreg] = {							\
+	.desc = {							\
+		.name = #vreg,						\
+		.of_match = of_match_ptr(match),			\
+		.ops = &mt6380_volt_table_ops,				\
+		.type = REGULATOR_VOLTAGE,				\
+		.id = MT6380_ID_##vreg,					\
+		.owner = THIS_MODULE,					\
+		.n_voltages = ARRAY_SIZE(ldo_volt_table),		\
+		.volt_table = ldo_volt_table,				\
+		.vsel_reg = vosel,					\
+		.vsel_mask = vosel_mask,				\
+		.enable_reg = enreg,					\
+		.enable_mask = BIT(enbit),				\
+	},								\
+	.modeset_reg = _modeset_reg,					\
+	.modeset_mask = _modeset_mask,					\
+}
+
+#define MT6380_REG_FIXED(match, vreg, enreg, enbit, volt,		\
+		_modeset_reg, _modeset_mask)				\
+[MT6380_ID_##vreg] = {							\
+	.desc = {							\
+		.name = #vreg,						\
+		.of_match = of_match_ptr(match),			\
+		.ops = &mt6380_volt_fixed_ops,				\
+		.type = REGULATOR_VOLTAGE,				\
+		.id = MT6380_ID_##vreg,					\
+		.owner = THIS_MODULE,					\
+		.n_voltages = 1,					\
+		.enable_reg = enreg,					\
+		.enable_mask = BIT(enbit),				\
+		.min_uV = volt,						\
+	},								\
+	.modeset_reg = _modeset_reg,					\
+	.modeset_mask = _modeset_mask,					\
+}
+
+static const struct regulator_linear_range buck_volt_range1[] = {
+	REGULATOR_LINEAR_RANGE(600000, 0, 0xfe, 6250),
+};
+
+static const struct regulator_linear_range buck_volt_range2[] = {
+	REGULATOR_LINEAR_RANGE(600000, 0, 0xfe, 6250),
+};
+
+static const struct regulator_linear_range buck_volt_range3[] = {
+	REGULATOR_LINEAR_RANGE(1200000, 0, 0x3c, 25000),
+};
+
+static const u32 ldo_volt_table1[] = {
+	1400000, 1350000, 1300000, 1250000, 1200000, 1150000, 1100000, 1050000,
+};
+
+static const u32 ldo_volt_table2[] = {
+	2200000, 3300000,
+};
+
+static const u32 ldo_volt_table3[] = {
+	1240000, 1390000, 1540000, 1840000,
+};
+
+static const u32 ldo_volt_table4[] = {
+	2200000, 3300000,
+};
+
+static int mt6380_get_status(struct regulator_dev *rdev)
+{
+	int ret;
+	u32 regval;
+	struct mt6380_regulator_info *info = rdev_get_drvdata(rdev);
+
+	ret = regmap_read(rdev->regmap, info->desc.enable_reg, &regval);
+	if (ret != 0) {
+		dev_err(&rdev->dev, "Failed to get enable reg: %d\n", ret);
+		return ret;
+	}
+
+	return (regval & info->desc.enable_mask) ?
+		REGULATOR_STATUS_ON : REGULATOR_STATUS_OFF;
+}
+
+static int mt6380_regulator_set_mode(struct regulator_dev *rdev,
+				     unsigned int mode)
+{
+	int ret, val = 0;
+	struct mt6380_regulator_info *info = rdev_get_drvdata(rdev);
+	u32 reg_value;
+
+	if (!info->modeset_mask) {
+		dev_err(&rdev->dev, "regulator %s doesn't support set_mode\n",
+			info->desc.name);
+		return -EINVAL;
+	}
+
+	switch (mode) {
+	case REGULATOR_MODE_STANDBY:
+		val = MT6380_REGULATOR_MODE_LP;
+		break;
+	case REGULATOR_MODE_NORMAL:
+		val = MT6380_REGULATOR_MODE_NORMAL;
+		break;
+	case REGULATOR_MODE_FAST:
+		val = MT6380_REGULATOR_MODE_FORCE_PWM;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	val <<= ffs(info->modeset_mask) - 1;
+
+	ret = regmap_update_bits(rdev->regmap, info->modeset_reg,
+				 info->modeset_mask, val);
+
+	if (regmap_read(rdev->regmap, info->modeset_reg, &reg_value) < 0) {
+		dev_err(&rdev->dev, "Failed to read register value\n");
+		return -EIO;
+	}
+
+	dev_dbg(&rdev->dev, "info->modeset_reg 0x%x = 0x%x\n",
+		info->modeset_reg, reg_value);
+
+	return ret;
+}
+
+static unsigned int mt6380_regulator_get_mode(struct regulator_dev *rdev)
+{
+	unsigned int val;
+	unsigned int mode;
+	int ret;
+	struct mt6380_regulator_info *info = rdev_get_drvdata(rdev);
+
+	if (!info->modeset_mask) {
+		dev_err(&rdev->dev, "regulator %s doesn't support get_mode\n",
+			info->desc.name);
+		return -EINVAL;
+	}
+
+	ret = regmap_read(rdev->regmap, info->modeset_reg, &val);
+	if (ret < 0)
+		return ret;
+
+	val &= info->modeset_mask;
+	val >>= ffs(info->modeset_mask) - 1;
+
+	if (val & 0x1)
+		mode = REGULATOR_MODE_STANDBY;
+	else
+		mode = REGULATOR_MODE_NORMAL;
+
+	return mode;
+}
+
+static struct regulator_ops mt6380_volt_range_ops = {
+	.list_voltage = regulator_list_voltage_linear_range,
+	.map_voltage = regulator_map_voltage_linear_range,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.set_voltage_time_sel = regulator_set_voltage_time_sel,
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.get_status = mt6380_get_status,
+	.set_mode = mt6380_regulator_set_mode,
+	.get_mode = mt6380_regulator_get_mode,
+};
+
+static struct regulator_ops mt6380_volt_table_ops = {
+	.list_voltage = regulator_list_voltage_table,
+	.map_voltage = regulator_map_voltage_iterate,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.set_voltage_time_sel = regulator_set_voltage_time_sel,
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.get_status = mt6380_get_status,
+	.set_mode = mt6380_regulator_set_mode,
+	.get_mode = mt6380_regulator_get_mode,
+};
+
+static struct regulator_ops mt6380_volt_fixed_ops = {
+	.list_voltage = regulator_list_voltage_linear,
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.get_status = mt6380_get_status,
+	.set_mode = mt6380_regulator_set_mode,
+	.get_mode = mt6380_regulator_get_mode,
+};
+
+/* The array is indexed by id(MT6380_ID_XXX) */
+static struct mt6380_regulator_info mt6380_regulators[] = {
+	MT6380_BUCK("buck_vcore1", VCPU, 600000, 1393750, 6250,
+		    buck_volt_range1, MT6380_ANA_CTRL_3, MT6380_ANA_CTRL_1,
+		    0xfe, 3, MT6380_ANA_CTRL_1, MT6380_CPUBUCK_CON_0,
+		    0x8000000),
+	MT6380_BUCK("buck_vcore", VCORE, 600000, 1393750, 6250,
+		    buck_volt_range2, MT6380_ANA_CTRL_3, MT6380_ANA_CTRL_2,
+		    0xfe, 2, MT6380_ANA_CTRL_2, 0, 0),
+	MT6380_BUCK("buck_vrf", VRF, 1200000, 1575000, 25000,
+		    buck_volt_range3, MT6380_ANA_CTRL_3, MT6380_SIDO_CON_0,
+		    0x78, 1, MT6380_SIDO_CON_0, 0, 0),
+	MT6380_LDO("ldo_vmldo", VMLDO, ldo_volt_table1, MT6380_LDO_CTRL_0,
+		   1, MT6380_MLDO_CON_0, 0xE000, MT6380_ANA_CTRL_1, 0x4000000),
+	MT6380_LDO("ldo_valdo", VALDO, ldo_volt_table2, MT6380_LDO_CTRL_0,
+		   2, MT6380_ALDO_CON_0, 0x400, MT6380_ALDO_CON_0, 0x20),
+	MT6380_REG_FIXED("ldo_vphyldo", VPHYLDO, MT6380_LDO_CTRL_0, 7, 1800000,
+			 MT6380_PHYLDO_CON_0, 0x80),
+	MT6380_LDO("ldo_vddrldo", VDDRLDO, ldo_volt_table3, MT6380_LDO_CTRL_0,
+		   8, MT6380_DDRLDO_CON_0, 0x3000, MT6380_DDRLDO_CON_0, 0x80),
+	MT6380_LDO("ldo_vtldo", VTLDO, ldo_volt_table4, MT6380_LDO_CTRL_0,
+		   3, MT6380_TLDO_CON_0, 0x400, MT6380_TLDO_CON_0, 0x20),
+};
+
+static int mt6380_regulator_probe(struct platform_device *pdev)
+{
+	struct regmap *regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	struct regulator_config config = {};
+	struct regulator_dev *rdev;
+	struct regulation_constraints *c;
+	int i;
+
+	for (i = 0; i < MT6380_MAX_REGULATOR; i++) {
+		config.dev = &pdev->dev;
+		config.driver_data = &mt6380_regulators[i];
+		config.regmap = regmap;
+		rdev = devm_regulator_register(&pdev->dev,
+					       &mt6380_regulators[i].desc,
+					       &config);
+		if (IS_ERR(rdev)) {
+			dev_err(&pdev->dev, "failed to register %s\n",
+				mt6380_regulators[i].desc.name);
+			return PTR_ERR(rdev);
+		}
+		/* Constrain board-specific capabilities according to what
+		 * this driver and the chip itself can actually do.
+		 */
+		c = rdev->constraints;
+		c->valid_modes_mask |= REGULATOR_MODE_NORMAL |
+			REGULATOR_MODE_STANDBY | REGULATOR_MODE_FAST;
+		c->valid_ops_mask |= REGULATOR_CHANGE_MODE;
+	}
+	return 0;
+}
+
+static const struct platform_device_id mt6380_platform_ids[] = {
+	{"mt6380-regulator", 0},
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(platform, mt6380_platform_ids);
+
+static const struct of_device_id mt6380_of_match[] = {
+	{ .compatible = "mediatek,mt6380-regulator", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, mt6380_of_match);
+
+static struct platform_driver mt6380_regulator_driver = {
+	.driver = {
+		.name = "mt6380-regulator",
+		.of_match_table = of_match_ptr(mt6380_of_match),
+	},
+	.probe = mt6380_regulator_probe,
+	.id_table = mt6380_platform_ids,
+};
+
+module_platform_driver(mt6380_regulator_driver);
+
+MODULE_AUTHOR("Chenglin Xu <chenglin.xu@mediatek.com>");
+MODULE_DESCRIPTION("Regulator Driver for MediaTek MT6380 PMIC");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/regulator/mt6380-regulator.h b/include/linux/regulator/mt6380-regulator.h
new file mode 100644
index 0000000..465182d
--- /dev/null
+++ b/include/linux/regulator/mt6380-regulator.h
@@ -0,0 +1,32 @@
+/*
+ * Copyright (c) 2017 MediaTek Inc.
+ * Author: Chenglin Xu <chenglin.xu@mediatek.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __LINUX_REGULATOR_mt6380_H
+#define __LINUX_REGULATOR_mt6380_H
+
+enum {
+	MT6380_ID_VCPU = 0,
+	MT6380_ID_VCORE,
+	MT6380_ID_VRF,
+	MT6380_ID_VMLDO,
+	MT6380_ID_VALDO,
+	MT6380_ID_VPHYLDO,
+	MT6380_ID_VDDRLDO,
+	MT6380_ID_VTLDO,
+	MT6380_ID_RG_MAX,
+};
+
+#define MT6380_MAX_REGULATOR	MT6380_ID_RG_MAX
+
+#endif /* __LINUX_REGULATOR_mt6380_H */
-- 
1.9.1

[PATCH 4/9] soc: mediatek: pwrap: add pwrap_read32 for reading in 32-bit mode

[sean.wang]

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

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@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 | 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 a5f1093..4385e8d 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -487,6 +487,7 @@ enum pwrap_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 irqreturn_t pwrap_interrupt(int irqno, void *dev_id)
 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[] = {
-- 
1.9.1

[PATCH 5/9] 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 4385e8d..03c0520 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 irqreturn_t pwrap_interrupt(int irqno, void *dev_id)
 	.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[] = {
-- 
1.9.1

[PATCH 6/9] soc: mediatek: pwrap: add pwrap_init pointer decided by the actual PMIC

[sean.wang]

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

pwrap initialization is highly associated with the PMIC types, so
one additional function pointer in struct pwrap_slv_type is introduced for
increasing flexibility and selecting the proper way when the slave device
is detected.

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 | 46 +++++++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index 03c0520..d9b4a63 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -505,6 +505,7 @@ struct pwrap_slv_type {
 	 * so the pointers added increases flexibility allowing determination
 	 * which type is used by the detection through device tree.
 	 */
+	int (*pwrap_init)(struct pmic_wrapper *wrp);
 	int (*pwrap_read)(struct pmic_wrapper *wrp, u32 adr, u32 *rdata);
 	int (*pwrap_write)(struct pmic_wrapper *wrp, u32 adr, u32 wdata);
 };
@@ -983,7 +984,7 @@ static int pwrap_mt2701_init_soc_specific(struct pmic_wrapper *wrp)
 	return 0;
 }
 
-static int pwrap_init(struct pmic_wrapper *wrp)
+static int pwrap_init_mt6397(struct pmic_wrapper *wrp)
 {
 	int ret;
 	u32 rdata;
@@ -1081,6 +1082,43 @@ static int pwrap_init(struct pmic_wrapper *wrp)
 	return 0;
 }
 
+static int pwrap_init_mt6380(struct pmic_wrapper *wrp)
+{
+	int ret;
+
+	reset_control_reset(wrp->rstc);
+
+	pwrap_writel(wrp, 1, PWRAP_WRAP_EN);
+	pwrap_writel(wrp, wrp->master->arb_en_all, PWRAP_HIPRIO_ARB_EN);
+	pwrap_writel(wrp, 1, PWRAP_WACS2_EN);
+
+	ret = wrp->master->init_reg_clock(wrp);
+	if (ret)
+		return ret;
+
+	pwrap_writel(wrp, 0x1, PWRAP_WACS0_EN);
+	pwrap_writel(wrp, 0x1, PWRAP_WACS1_EN);
+	pwrap_writel(wrp, 0x1, PWRAP_WACS2_EN);
+
+	if (wrp->master->init_soc_specific) {
+		ret = wrp->master->init_soc_specific(wrp);
+		if (ret)
+			return ret;
+	}
+
+	/* Setup the init done registers */
+	pwrap_writel(wrp, 1, PWRAP_INIT_DONE2);
+	pwrap_writel(wrp, 1, PWRAP_INIT_DONE0);
+	pwrap_writel(wrp, 1, PWRAP_INIT_DONE1);
+
+	return 0;
+}
+
+static int pwrap_init(struct pmic_wrapper *wrp)
+{
+	return wrp->slave->pwrap_init(wrp);
+}
+
 static irqreturn_t pwrap_interrupt(int irqno, void *dev_id)
 {
 	u32 rdata;
@@ -1107,6 +1145,8 @@ static irqreturn_t pwrap_interrupt(int irqno, void *dev_id)
 static const struct pwrap_slv_type pmic_mt6323 = {
 	.dew_regs = mt6323_regs,
 	.type = PMIC_MT6323,
+
+	.pwrap_init = pwrap_init_mt6397,
 	.pwrap_read = pwrap_read16,
 	.pwrap_write = pwrap_write16,
 };
@@ -1114,6 +1154,8 @@ static irqreturn_t pwrap_interrupt(int irqno, void *dev_id)
 static const struct pwrap_slv_type pmic_mt6380 = {
 	.dew_regs = NULL,
 	.type = PMIC_MT6380,
+
+	.pwrap_init = pwrap_init_mt6380,
 	.pwrap_read = pwrap_read32,
 	.pwrap_write = pwrap_write32,
 };
@@ -1121,6 +1163,8 @@ static irqreturn_t pwrap_interrupt(int irqno, void *dev_id)
 static const struct pwrap_slv_type pmic_mt6397 = {
 	.dew_regs = mt6397_regs,
 	.type = PMIC_MT6397,
+
+	.pwrap_init = pwrap_init_mt6397,
 	.pwrap_read = pwrap_read16,
 	.pwrap_write = pwrap_write16,
 };
-- 
1.9.1

[PATCH 7/9] 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 | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index d9b4a63..90b087f 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -501,6 +501,7 @@ enum pwrap_type {
 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.
@@ -1133,7 +1134,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,
@@ -1142,10 +1143,19 @@ static irqreturn_t pwrap_interrupt(int irqno, void *dev_id)
 	.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_init = pwrap_init_mt6397,
 	.pwrap_read = pwrap_read16,
 	.pwrap_write = pwrap_write16,
@@ -1154,7 +1164,7 @@ static irqreturn_t pwrap_interrupt(int irqno, void *dev_id)
 static const struct pwrap_slv_type pmic_mt6380 = {
 	.dew_regs = NULL,
 	.type = PMIC_MT6380,
-
+	.regmap = &pwrap_regmap_config32,
 	.pwrap_init = pwrap_init_mt6380,
 	.pwrap_read = pwrap_read32,
 	.pwrap_write = pwrap_write32,
@@ -1163,7 +1173,7 @@ static irqreturn_t pwrap_interrupt(int irqno, void *dev_id)
 static const struct pwrap_slv_type pmic_mt6397 = {
 	.dew_regs = mt6397_regs,
 	.type = PMIC_MT6397,
-
+	.regmap = &pwrap_regmap_config16,
 	.pwrap_init = pwrap_init_mt6397,
 	.pwrap_read = pwrap_read16,
 	.pwrap_write = pwrap_write16,
@@ -1177,6 +1187,13 @@ static irqreturn_t pwrap_interrupt(int irqno, void *dev_id)
 		.compatible = "mediatek,mt6397",
 		.data = &pmic_mt6397,
 	}, {
+		/* The MT6380 slave device is directly pointed to regulator
+		 * device which is different from the cases MT6323 and MT6397
+		 * since they're one kind of MFDs.
+		 */
+		.compatible = "mediatek,mt6380-regulator",
+		.data = &pmic_mt6380,
+	}, {
 		/* sentinel */
 	}
 };
@@ -1346,7 +1363,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))
 		return PTR_ERR(wrp->regmap);
 
-- 
1.9.1

[PATCH 8/9] soc: mediatek: pwrap: add support for MT7622 SoC

[sean.wang]

From: Chenglin Xu <chenglin.xu@mediatek.com>

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

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 | 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 90b087f..6dc3038 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -242,6 +242,36 @@ enum pwrap_regs {
 	PWRAP_DVFS_WDATA7,
 	PWRAP_SPMINF_STA,
 	PWRAP_CIPHER_EN,
+
+	/* 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,
 };
 
 static int mt2701_regs[] = {
@@ -485,6 +515,118 @@ enum pwrap_regs {
 	[PWRAP_DCM_DBC_PRD] =		0x160,
 };
 
+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,
+};
+
 enum pmic_type {
 	PMIC_MT6323,
 	PMIC_MT6380,
@@ -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_mt6397(struct pmic_wrapper *wrp)
 {
 	int ret;
@@ -1235,6 +1400,18 @@ static irqreturn_t pwrap_interrupt(int irqno, void *dev_id)
 	.init_soc_specific = pwrap_mt8173_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 struct of_device_id of_pwrap_match_tbl[] = {
 	{
 		.compatible = "mediatek,mt2701-pwrap",
@@ -1246,6 +1423,9 @@ static irqreturn_t pwrap_interrupt(int irqno, void *dev_id)
 		.compatible = "mediatek,mt8173-pwrap",
 		.data = &pwrap_mt8173,
 	}, {
+		.compatible = "mediatek,mt7622-pwrap",
+		.data = &pwrap_mt7622,
+	}, {
 		/* sentinel */
 	}
 };
-- 
1.9.1

[PATCH 9/9] 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 | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index 6dc3038..93d3193 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;
 	}
 
@@ -1200,8 +1201,9 @@ static int pwrap_init_mt6397(struct pmic_wrapper *wrp)
 	/* 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",
-				PWRAP_DEW_READ_TEST_VAL, rdata);
+		dev_err(wrp->dev,
+			"Read test failed on DIO mode: 0x%04x!=0x%04x\n",
+			PWRAP_DEW_READ_TEST_VAL, rdata);
 		return -EFAULT;
 	}
 
@@ -1478,23 +1480,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);
 	}
 
@@ -1538,8 +1544,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;
 
-- 
1.9.1

Re: [PATCH 3/9] regulator: mt6380: Add support for MT6380

[Mark Brown]

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

On Sat, Jun 03, 2017 at 01:55:44AM +0800, sean.wang@mediatek.com wrote:

> +static int mt6380_get_status(struct regulator_dev *rdev)
> +{
> +	int ret;
> +	u32 regval;
> +	struct mt6380_regulator_info *info = rdev_get_drvdata(rdev);
> +
> +	ret = regmap_read(rdev->regmap, info->desc.enable_reg, &regval);
> +	if (ret != 0) {
> +		dev_err(&rdev->dev, "Failed to get enable reg: %d\n", ret);
> +		return ret;
> +	}
> +
> +	return (regval & info->desc.enable_mask) ?
> +		REGULATOR_STATUS_ON : REGULATOR_STATUS_OFF;

This isn't really a get_status() operation - it's just showing the
status of the enable we set.  The get_status() operation is for hardware
that has a mechanism for reading back the current physical status of the
regulator, usually including things like if it's in regulation or not.

Also please write normal conditional statements, it helps people read
the code.

> +	ret = regmap_update_bits(rdev->regmap, info->modeset_reg,
> +				 info->modeset_mask, val);
> +
> +	if (regmap_read(rdev->regmap, info->modeset_reg, &reg_value) < 0) {
> +		dev_err(&rdev->dev, "Failed to read register value\n");
> +		return -EIO;
> +	}

Is I/O to the device unreliable for some reason?  If so this isn't great
handling for it...  also if there is an error from regmap_read() you
should return the error code.

> +	unsigned int mode;
> +	int ret;
> +	struct mt6380_regulator_info *info = rdev_get_drvdata(rdev);
> +
> +	if (!info->modeset_mask) {
> +		dev_err(&rdev->dev, "regulator %s doesn't support get_mode\n",
> +			info->desc.name);
> +		return -EINVAL;
> +	}
> +
> +	ret = regmap_read(rdev->regmap, info->modeset_reg, &val);
> +	if (ret < 0)
> +		return ret;
> +
> +	val &= info->modeset_mask;
> +	val >>= ffs(info->modeset_mask) - 1;
> +
> +	if (val & 0x1)
> +		mode = REGULATOR_MODE_STANDBY;
> +	else
> +		mode = REGULATOR_MODE_NORMAL;
> +
> +	return mode;
> +}

This won't initialize mode if the regulator is in force PWM mode.  It'd
be clearer and safer to just write a switch statement.

> +		/* Constrain board-specific capabilities according to what
> +		 * this driver and the chip itself can actually do.
> +		 */
> +		c = rdev->constraints;
> +		c->valid_modes_mask |= REGULATOR_MODE_NORMAL |
> +			REGULATOR_MODE_STANDBY | REGULATOR_MODE_FAST;

No, this is completely broken.  A regulator driver should *never* modify
constraints, if the constraints are broken the constraints are broken,
and the constraints will already have been applied when we return from
registering the regulator.

> +		c->valid_ops_mask |= REGULATOR_CHANGE_MODE;

> +static const struct of_device_id mt6380_of_match[] = {
> +	{ .compatible = "mediatek,mt6380-regulator", },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, mt6380_of_match);

Given that this driver is entirely specific to the parent PMIC there
should be no need for a separate compatible string, it's redundant.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Rob Herring]

On Sat, Jun 03, 2017 at 01:55:42AM +0800, sean.wang@mediatek.com wrote:
> 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>
> ---
>  Documentation/devicetree/bindings/soc/mediatek/pwrap.txt | 4 ++++
>  1 file changed, 4 insertions(+)

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 2/9] dt-bindings: regulator: Add document for MediaTek MT6380 regulator

[Rob Herring]

On Sat, Jun 03, 2017 at 01:55:43AM +0800, sean.wang@mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
> 
> add dt-binding document for MediaTek MT6380 PMIC
> 
> Signed-off-by: Chenglin Xu <chenglin.xu@mediatek.com>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
>  .../bindings/regulator/mt6380-regulator.txt        | 90 ++++++++++++++++++++++
>  1 file changed, 90 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/mt6380-regulator.txt
> 
> diff --git a/Documentation/devicetree/bindings/regulator/mt6380-regulator.txt b/Documentation/devicetree/bindings/regulator/mt6380-regulator.txt
> new file mode 100644
> index 0000000..9affeab
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/mt6380-regulator.txt
> @@ -0,0 +1,90 @@
> +Mediatek MT6380 Regulator Driver
> +
> +All voltage regulators are defined as subnodes of the regulators node. A list
> +of regulators provided by this controller are defined as subnodes of the
> +PMIC's node. Each regulator is named according to its regulator type,
> +buck_<name> and ldo_<name>. The definition for each of these nodes is defined
> +using the standard binding for regulators at
> +Documentation/devicetree/bindings/regulator/regulator.txt.
> +
> +The valid names for regulators are::
> +BUCK:
> +  buck_vcore1, buck_vcore, buck_vrf
> +LDO:
> +  ldo_vmldo ,ldo_valdo , ldo_vphyldo, ldo_vddrldo, ldo_vtldo

'ldo' twice in every name is redundant.

Use of '_' is discouraged, but if that's what prior PMICs did, it's 
okay.

> +
> +Example:
> +
> +	mt6380regulator: mt6380regulator {

just "regulators {"

> +		compatible = "mediatek,mt6380-regulator";
> +
> +		mt6380_vcpu_reg: buck_vcore1 {
> +			regulator-name = "vcpu";
> +			regulator-min-microvolt = < 600000>;
> +			regulator-max-microvolt = <1393750>;
> +			regulator-ramp-delay = <6250>;
> +			regulator-always-on;
> +			regulator-boot-on;
> +		};
> +
> +		mt6380_vcore_reg: buck_vcore {
> +			regulator-name = "vcore";
> +			regulator-min-microvolt = <600000>;
> +			regulator-max-microvolt = <1393750>;
> +			regulator-ramp-delay = <6250>;
> +		};
> +
> +		mt6380_vrf_reg: buck_vrf {
> +			regulator-name = "vrf";
> +			regulator-min-microvolt = <1200000>;
> +			regulator-max-microvolt = <1575000>;
> +			regulator-ramp-delay = <0>;
> +			regulator-always-on;
> +			regulator-boot-on;
> +		};
> +
> +		mt6380_vm_reg: ldo_vmldo {
> +			regulator-name = "vmldo";
> +			regulator-min-microvolt = <1050000>;
> +			regulator-max-microvolt = <1400000>;
> +			regulator-ramp-delay = <0>;
> +			regulator-always-on;
> +			regulator-boot-on;
> +		};
> +
> +		mt6380_va_reg: ldo_valdo {
> +			regulator-name = "valdo";
> +			regulator-min-microvolt = <2200000>;
> +			regulator-max-microvolt = <3300000>;
> +			regulator-ramp-delay = <0>;
> +			regulator-always-on;
> +			regulator-boot-on;
> +		};
> +
> +		mt6380_vphy_reg: ldo_vphyldo {
> +			regulator-name = "vphyldo";
> +			regulator-min-microvolt = <1800000>;
> +			regulator-max-microvolt = <1800000>;
> +			regulator-ramp-delay = <0>;
> +			regulator-always-on;
> +			regulator-boot-on;
> +		};
> +
> +		mt6380_vddr_reg: ldo_vddrldo {
> +			regulator-name = "vddr";
> +			regulator-min-microvolt = <1240000>;
> +			regulator-max-microvolt = <1840000>;
> +			regulator-ramp-delay = <0>;
> +			regulator-always-on;
> +			regulator-boot-on;
> +		};
> +
> +		mt6380_vt_reg: ldo_vtldo {
> +			regulator-name = "vadc18";
> +			regulator-min-microvolt = <2200000>;
> +			regulator-max-microvolt = <3300000>;
> +			regulator-ramp-delay = <0>;
> +			regulator-always-on;
> +			regulator-boot-on;
> +		};
> +	};
> -- 
> 1.9.1
> 

Re: [PATCH 3/9] regulator: mt6380: Add support for MT6380

[Sean Wang]

Hi Mark,

appreciate your effort on reviewing those patches 

we'll make next version following your suggestion, here also adding some
comments as inline to explain what thoughts in mind

On Tue, 2017-06-06 at 19:22 +0100, Mark Brown wrote:
> On Sat, Jun 03, 2017 at 01:55:44AM +0800, sean.wang@mediatek.com wrote:
> 
> > +static int mt6380_get_status(struct regulator_dev *rdev)
> > +{
> > +	int ret;
> > +	u32 regval;
> > +	struct mt6380_regulator_info *info = rdev_get_drvdata(rdev);
> > +
> > +	ret = regmap_read(rdev->regmap, info->desc.enable_reg, &regval);
> > +	if (ret != 0) {
> > +		dev_err(&rdev->dev, "Failed to get enable reg: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	return (regval & info->desc.enable_mask) ?
> > +		REGULATOR_STATUS_ON : REGULATOR_STATUS_OFF;
> 
> This isn't really a get_status() operation - it's just showing the
> status of the enable we set.  The get_status() operation is for hardware
> that has a mechanism for reading back the current physical status of the
> regulator, usually including things like if it's in regulation or not.
> 
> Also please write normal conditional statements, it helps people read
> the code.

for the hardware, the way for reflect the current physical physical 
has to depend on the bit reading as the bit we enable. It indeed tends
to confuse other users and developers, we maybe can add some comments
for this to avoid.


> > +	ret = regmap_update_bits(rdev->regmap, info->modeset_reg,
> > +				 info->modeset_mask, val);
> > +
> > +	if (regmap_read(rdev->regmap, info->modeset_reg, &reg_value) < 0) {
> > +		dev_err(&rdev->dev, "Failed to read register value\n");
> > +		return -EIO;
> > +	}
> 
> Is I/O to the device unreliable for some reason?  If so this isn't great
> handling for it...  also if there is an error from regmap_read() you
> should return the error code.
> 

O.K. we'll remove it. that just is debug purpose code which confirms the
value we set is correct.


> > +	unsigned int mode;
> > +	int ret;
> > +	struct mt6380_regulator_info *info = rdev_get_drvdata(rdev);
> > +
> > +	if (!info->modeset_mask) {
> > +		dev_err(&rdev->dev, "regulator %s doesn't support get_mode\n",
> > +			info->desc.name);
> > +		return -EINVAL;
> > +	}
> > +
> > +	ret = regmap_read(rdev->regmap, info->modeset_reg, &val);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	val &= info->modeset_mask;
> > +	val >>= ffs(info->modeset_mask) - 1;
> > +
> > +	if (val & 0x1)
> > +		mode = REGULATOR_MODE_STANDBY;
> > +	else
> > +		mode = REGULATOR_MODE_NORMAL;
> > +
> > +	return mode;
> > +}
> 
> This won't initialize mode if the regulator is in force PWM mode.  It'd
> be clearer and safer to just write a switch statement.
> 

we'll fix up the bug with switch statement.

> > +		/* Constrain board-specific capabilities according to what
> > +		 * this driver and the chip itself can actually do.
> > +		 */
> > +		c = rdev->constraints;
> > +		c->valid_modes_mask |= REGULATOR_MODE_NORMAL |
> > +			REGULATOR_MODE_STANDBY | REGULATOR_MODE_FAST;
> 
> No, this is completely broken.  A regulator driver should *never* modify
> constraints, if the constraints are broken the constraints are broken,
> and the constraints will already have been applied when we return from
> registering the regulator.
> 

agreed. we shouldn't break any constraint and the violation would be
removed.

> > +		c->valid_ops_mask |= REGULATOR_CHANGE_MODE;
> 
> > +static const struct of_device_id mt6380_of_match[] = {
> > +	{ .compatible = "mediatek,mt6380-regulator", },
> > +	{ /* sentinel */ },
> > +};
> > +MODULE_DEVICE_TABLE(of, mt6380_of_match);
> 
> Given that this driver is entirely specific to the parent PMIC there
> should be no need for a separate compatible string, it's redundant.


the parent of pmic is MediaTek pwrap which is possibly being used with
various pmics such as MT6323, MT6797, MT6380 and so on. So extra
matching we thought is required to identify which pmic is actually being
connected. 

For those opinions, maybe we didn't get your exact point. If something
is wrong, please kindly guide us to the right place.

	Sean

Re: [PATCH 3/9] regulator: mt6380: Add support for MT6380

[Mark Brown]

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

On Fri, Jun 23, 2017 at 11:56:05PM +0800, Sean Wang wrote:
> On Tue, 2017-06-06 at 19:22 +0100, Mark Brown wrote:

> > > +	return (regval & info->desc.enable_mask) ?
> > > +		REGULATOR_STATUS_ON : REGULATOR_STATUS_OFF;

> > This isn't really a get_status() operation - it's just showing the
> > status of the enable we set.  The get_status() operation is for hardware
> > that has a mechanism for reading back the current physical status of the
> > regulator, usually including things like if it's in regulation or not.

> > Also please write normal conditional statements, it helps people read
> > the code.

> for the hardware, the way for reflect the current physical physical 
> has to depend on the bit reading as the bit we enable. It indeed tends
> to confuse other users and developers, we maybe can add some comments
> for this to avoid.

It's OK to just not have a get_status() operation - a lot of regulators
just can't do this and that's fine, the subsystem will cope.

> > > +static const struct of_device_id mt6380_of_match[] = {
> > > +	{ .compatible = "mediatek,mt6380-regulator", },
> > > +	{ /* sentinel */ },
> > > +};
> > > +MODULE_DEVICE_TABLE(of, mt6380_of_match);

> > Given that this driver is entirely specific to the parent PMIC there
> > should be no need for a separate compatible string, it's redundant.

> the parent of pmic is MediaTek pwrap which is possibly being used with
> various pmics such as MT6323, MT6797, MT6380 and so on. So extra
> matching we thought is required to identify which pmic is actually being
> connected. 

> For those opinions, maybe we didn't get your exact point. If something
> is wrong, please kindly guide us to the right place.

It sounds like pwrap should be a bus rather than using a platform device
here?  But I guess that's how things are for now so OK.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 3/9] regulator: mt6380: Add support for MT6380

[Sean Wang]

On Fri, 2017-06-23 at 17:14 +0100, Mark Brown wrote:
> On Fri, Jun 23, 2017 at 11:56:05PM +0800, Sean Wang wrote:
> > On Tue, 2017-06-06 at 19:22 +0100, Mark Brown wrote:
> 
> > > > +	return (regval & info->desc.enable_mask) ?
> > > > +		REGULATOR_STATUS_ON : REGULATOR_STATUS_OFF;
> 
> > > This isn't really a get_status() operation - it's just showing the
> > > status of the enable we set.  The get_status() operation is for hardware
> > > that has a mechanism for reading back the current physical status of the
> > > regulator, usually including things like if it's in regulation or not.
> 
> > > Also please write normal conditional statements, it helps people read
> > > the code.
> 
> > for the hardware, the way for reflect the current physical physical 
> > has to depend on the bit reading as the bit we enable. It indeed tends
> > to confuse other users and developers, we maybe can add some comments
> > for this to avoid.
> 
> It's OK to just not have a get_status() operation - a lot of regulators
> just can't do this and that's fine, the subsystem will cope.
> 

understood. it seems to be better with subsystem coping. we'll remove
get_status callback.

> > > > +static const struct of_device_id mt6380_of_match[] = {
> > > > +	{ .compatible = "mediatek,mt6380-regulator", },
> > > > +	{ /* sentinel */ },
> > > > +};
> > > > +MODULE_DEVICE_TABLE(of, mt6380_of_match);
> 
> > > Given that this driver is entirely specific to the parent PMIC there
> > > should be no need for a separate compatible string, it's redundant.
> 
> > the parent of pmic is MediaTek pwrap which is possibly being used with
> > various pmics such as MT6323, MT6797, MT6380 and so on. So extra
> > matching we thought is required to identify which pmic is actually being
> > connected. 
> 
> > For those opinions, maybe we didn't get your exact point. If something
> > is wrong, please kindly guide us to the right place.
> 
> It sounds like pwrap should be a bus rather than using a platform device
> here?  But I guess that's how things are for now so OK.

yes, it is a bus , a proprietary bus,  which is something like
encapsulation of spi and there's some protocol running on this 
between master/slave.

Re: [PATCH 3/9] regulator: mt6380: Add support for MT6380

[Sean Wang]

On Sat, 2017-06-24 at 00:43 +0800, Sean Wang wrote:
> On Fri, 2017-06-23 at 17:14 +0100, Mark Brown wrote:
> > On Fri, Jun 23, 2017 at 11:56:05PM +0800, Sean Wang wrote:
> > > On Tue, 2017-06-06 at 19:22 +0100, Mark Brown wrote:
> > 
> > > > > +	return (regval & info->desc.enable_mask) ?
> > > > > +		REGULATOR_STATUS_ON : REGULATOR_STATUS_OFF;
> > 
> > > > This isn't really a get_status() operation - it's just showing the
> > > > status of the enable we set.  The get_status() operation is for hardware
> > > > that has a mechanism for reading back the current physical status of the
> > > > regulator, usually including things like if it's in regulation or not.
> > 
> > > > Also please write normal conditional statements, it helps people read
> > > > the code.
> > 
> > > for the hardware, the way for reflect the current physical physical 
> > > has to depend on the bit reading as the bit we enable. It indeed tends
> > > to confuse other users and developers, we maybe can add some comments
> > > for this to avoid.
> > 
> > It's OK to just not have a get_status() operation - a lot of regulators
> > just can't do this and that's fine, the subsystem will cope.
> > 
> 
> understood. it seems to be better with subsystem coping. we'll remove
> get_status callback.
> 

Hi, Mark

We usually want to use /sys/class/regulator/regulator.*/status to get
actual status from hardware, but if we remove the get_status(), the
status entry under sysfs would be gone. So could I keep the get_status
callback in this driver ?

	Sean

> > > > > +static const struct of_device_id mt6380_of_match[] = {
> > > > > +	{ .compatible = "mediatek,mt6380-regulator", },
> > > > > +	{ /* sentinel */ },
> > > > > +};
> > > > > +MODULE_DEVICE_TABLE(of, mt6380_of_match);
> > 
> > > > Given that this driver is entirely specific to the parent PMIC there
> > > > should be no need for a separate compatible string, it's redundant.
> > 
> > > the parent of pmic is MediaTek pwrap which is possibly being used with
> > > various pmics such as MT6323, MT6797, MT6380 and so on. So extra
> > > matching we thought is required to identify which pmic is actually being
> > > connected. 
> > 
> > > For those opinions, maybe we didn't get your exact point. If something
> > > is wrong, please kindly guide us to the right place.
> > 
> > It sounds like pwrap should be a bus rather than using a platform device
> > here?  But I guess that's how things are for now so OK.
> 
> yes, it is a bus , a proprietary bus,  which is something like
> encapsulation of spi and there's some protocol running on this 
> between master/slave.
> 
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 3/9] regulator: mt6380: Add support for MT6380

[Mark Brown]

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

On Fri, Jul 14, 2017 at 11:06:08PM +0800, Sean Wang wrote:

> We usually want to use /sys/class/regulator/regulator.*/status to get
> actual status from hardware, but if we remove the get_status(), the
> status entry under sysfs would be gone. So could I keep the get_status
> callback in this driver ?

If the device doesn't support physical status readback it shouldn't have
a get_status() callback.  

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]