[PATCH v9 0/2] Add Maxim 77802 regulator support

[PATCH v9 0/2] Add Maxim 77802 regulator support

[Javier Martinez Canillas]

Hello Mark,

This series add support for the regulators present in the
Maxim 77802 Power Managment IC. Previously, the series was
part of a bigger one [0] that aimed to add support for all
the devices in the max77802 PMIC. But now the Maxim 77802
PMIC dependencies were already merged for 3.17 so the series
can be split and each driver can go through the relevant tree.

Patch 01/02 adds the driver for the regulators in the Maxim
77802 PMIC and patch 02/02 adds the related DT bindings doc.

I kept the version numbering from the old series so this is
version 9 although there were no changes on the max77802 driver
since v7 of the old series.

The series were tested on an Exynos5420 based Peach Pit board
and applies cleanly to both 3.17-rc1 and today's next-20140818.

Javier Martinez Canillas (2):
  regulator: Add driver for max77802 PMIC PMIC regulators
  regulator: Add DT bindings for max77802 PMIC regulators

 .../devicetree/bindings/regulator/max77802.txt     |  53 ++
 drivers/regulator/Kconfig                          |   9 +
 drivers/regulator/Makefile                         |   1 +
 drivers/regulator/max77802.c                       | 578 +++++++++++++++++++++
 4 files changed, 641 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/max77802.txt
 create mode 100644 drivers/regulator/max77802.c

Best regards,
Javier

[PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

[Javier Martinez Canillas]

The MAX77802 PMIC has 10 high-efficiency Buck and 32 Low-dropout
(LDO) regulators. This patch adds support for all these regulators
found on the MAX77802 PMIC and is based on a driver added by Simon
Glass to the Chrome OS kernel 3.8 tree.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Tested-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
---

Changes since v7:
 - Remove DVS support since that can be added as a follow up.

Changes since v6: None

Changes since v5:
 - Take out the mfd changes from v4 that were squashed by mistake.
   Suggested by Lee Jones.

Changes since v4: None

Changes since v3:
 - Set the supply_name for regulators to lookup their parent supply node.
   Suggested by Mark Brown.
 - Change Exyno5 for Exynos5420/Exynos5800 in regulator driver Kconfig.
   Suggested by Doug Anderson.
---
 drivers/regulator/Kconfig    |   9 +
 drivers/regulator/Makefile   |   1 +
 drivers/regulator/max77802.c | 578 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 588 insertions(+)
 create mode 100644 drivers/regulator/max77802.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 2dc8289..8134a99 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -387,6 +387,15 @@ config REGULATOR_MAX77693
 	  and one current regulator 'CHARGER'. This is suitable for
 	  Exynos-4x12 chips.
 
+config REGULATOR_MAX77802
+	tristate "Maxim 77802 regulator"
+	depends on MFD_MAX77686
+	help
+	  This driver controls a Maxim 77802 regulator
+	  via I2C bus. The provided regulator is suitable for
+	  Exynos5420/Exynos5800 SoCs to control various voltages.
+	  It includes support for control of voltage and ramp speed.
+
 config REGULATOR_MC13XXX_CORE
 	tristate
 
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index aa4a6aa..b4ec6c8 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -52,6 +52,7 @@ obj-$(CONFIG_REGULATOR_MAX8997) += max8997.o
 obj-$(CONFIG_REGULATOR_MAX8998) += max8998.o
 obj-$(CONFIG_REGULATOR_MAX77686) += max77686.o
 obj-$(CONFIG_REGULATOR_MAX77693) += max77693.o
+obj-$(CONFIG_REGULATOR_MAX77802) += max77802.o
 obj-$(CONFIG_REGULATOR_MC13783) += mc13783-regulator.o
 obj-$(CONFIG_REGULATOR_MC13892) += mc13892-regulator.o
 obj-$(CONFIG_REGULATOR_MC13XXX_CORE) +=  mc13xxx-regulator-core.o
diff --git a/drivers/regulator/max77802.c b/drivers/regulator/max77802.c
new file mode 100644
index 0000000..5f022f8
--- /dev/null
+++ b/drivers/regulator/max77802.c
@@ -0,0 +1,578 @@
+/*
+ * max77802.c - Regulator driver for the Maxim 77802
+ *
+ * Copyright (C) 2013-2014 Google, Inc
+ * Simon Glass <sjg@chromium.org>
+ *
+ * Copyright (C) 2012 Samsung Electronics
+ * Chiwoong Byun <woong.byun@smasung.com>
+ * Jonghwa Lee <jonghwa3.lee@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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.
+ *
+ * This driver is based on max8997.c
+ */
+
+#include <linux/kernel.h>
+#include <linux/bug.h>
+#include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/slab.h>
+#include <linux/gpio/consumer.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/mfd/max77686.h>
+#include <linux/mfd/max77686-private.h>
+
+/* Default ramp delay in case it is not manually set */
+#define MAX77802_RAMP_DELAY		100000		/* uV/us */
+
+#define MAX77802_OPMODE_SHIFT_LDO	6
+#define MAX77802_OPMODE_BUCK234_SHIFT	4
+#define MAX77802_OPMODE_MASK		0x3
+
+#define MAX77802_VSEL_MASK		0x3F
+#define MAX77802_DVS_VSEL_MASK		0xFF
+
+#define MAX77802_RAMP_RATE_MASK_2BIT	0xC0
+#define MAX77802_RAMP_RATE_SHIFT_2BIT	6
+#define MAX77802_RAMP_RATE_MASK_4BIT	0xF0
+#define MAX77802_RAMP_RATE_SHIFT_4BIT	4
+
+/* MAX77802 has two register formats: 2-bit and 4-bit */
+static const unsigned int ramp_table_77802_2bit[] = {
+	12500,
+	25000,
+	50000,
+	100000,
+};
+
+static unsigned int ramp_table_77802_4bit[] = {
+	1000,	2000,	3030,	4000,
+	5000,	5880,	7140,	8330,
+	9090,	10000,	11110,	12500,
+	16670,	25000,	50000,	100000,
+};
+
+struct max77802_regulator_prv {
+	int num_regulators;
+	struct regulator_dev *rdev[MAX77802_REG_MAX];
+	unsigned int opmode[MAX77802_REG_MAX];
+};
+
+static int max77802_get_opmode_shift(int id)
+{
+	if (id == MAX77802_BUCK1 || (id >= MAX77802_BUCK5 &&
+				     id <= MAX77802_BUCK10))
+		return 0;
+
+	if (id >= MAX77802_BUCK2 && id <= MAX77802_BUCK4)
+		return MAX77802_OPMODE_BUCK234_SHIFT;
+
+	if (id >= MAX77802_LDO1 && id <= MAX77802_LDO35)
+		return MAX77802_OPMODE_SHIFT_LDO;
+
+	return -EINVAL;
+}
+
+/*
+ * Some BUCKS supports Normal[ON/OFF] mode during suspend
+ *
+ * BUCK 1, 6, 2-4, 5, 7-10 (all)
+ *
+ * The other mode (0x02) will make PWRREQ switch between normal
+ * and low power.
+ */
+static int max77802_buck_set_suspend_disable(struct regulator_dev *rdev)
+{
+	unsigned int val = MAX77802_OPMODE_STANDBY;
+	struct max77802_regulator_prv *max77802 = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	int shift = max77802_get_opmode_shift(id);
+
+	max77802->opmode[id] = val;
+	return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
+				  rdev->desc->enable_mask, val << shift);
+}
+
+/*
+ * Some LDOs supports LPM-ON/OFF/Normal-ON mode during suspend state
+ * (Enable Control Logic1 by PWRREQ)
+ *
+ * LDOs 2, 4-19, 22-35.
+ *
+ */
+static int max77802_ldo_set_suspend_mode_logic1(struct regulator_dev *rdev,
+						unsigned int mode)
+{
+	struct max77802_regulator_prv *max77802 = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	unsigned int val;
+	int shift = max77802_get_opmode_shift(id);
+
+	switch (mode) {
+	case REGULATOR_MODE_IDLE:			/* ON in LP Mode */
+		val = MAX77802_OPMODE_LP;
+		break;
+	case REGULATOR_MODE_NORMAL:			/* ON in Normal Mode */
+		val = MAX77802_OPMODE_NORMAL;
+		break;
+	case REGULATOR_MODE_STANDBY:			/* ON/OFF by PWRREQ */
+		val = MAX77802_OPMODE_STANDBY;
+		break;
+	default:
+		dev_warn(&rdev->dev, "%s: regulator mode: 0x%x not supported\n",
+			 rdev->desc->name, mode);
+		return -EINVAL;
+	}
+
+	max77802->opmode[rdev_get_id(rdev)] = val;
+	return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
+				  rdev->desc->enable_mask, val << shift);
+}
+
+/*
+ * Mode 1 (Output[ON/OFF] by PWRREQ) is not supported on some LDOs
+ * (Enable Control Logic2 by PWRREQ)
+ *
+ * LDOs 1, 20, 21, and 3,
+ *
+ */
+static int max77802_ldo_set_suspend_mode_logic2(struct regulator_dev *rdev,
+						unsigned int mode)
+{
+	struct max77802_regulator_prv *max77802 = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	unsigned int val;
+	int shift = max77802_get_opmode_shift(id);
+
+	switch (mode) {
+	case REGULATOR_MODE_IDLE:			/* ON in LP Mode */
+		val = MAX77802_OPMODE_LP;
+		break;
+	case REGULATOR_MODE_NORMAL:			/* ON in Normal Mode */
+		val = MAX77802_OPMODE_NORMAL;
+		break;
+	default:
+		dev_warn(&rdev->dev, "%s: regulator mode: 0x%x not supported\n",
+			 rdev->desc->name, mode);
+		return -EINVAL;
+	}
+
+	max77802->opmode[rdev_get_id(rdev)] = val;
+	return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
+				  rdev->desc->enable_mask, val << shift);
+}
+
+static int max77802_enable(struct regulator_dev *rdev)
+{
+	struct max77802_regulator_prv *max77802 = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	int shift = max77802_get_opmode_shift(id);
+
+	return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
+				  rdev->desc->enable_mask,
+				  max77802->opmode[id] << shift);
+}
+
+static int max77802_find_ramp_value(struct regulator_dev *rdev,
+				    const unsigned int limits[], int size,
+				    unsigned int ramp_delay)
+{
+	int i;
+
+	for (i = 0; i < size; i++) {
+		if (ramp_delay <= limits[i])
+			return i;
+	}
+
+	/* Use maximum value for no ramp control */
+	dev_warn(&rdev->dev, "%s: ramp_delay: %d not supported, setting 100000\n",
+		 rdev->desc->name, ramp_delay);
+	return size - 1;
+}
+
+/* Used for BUCKs 2-4 */
+static int max77802_set_ramp_delay_2bit(struct regulator_dev *rdev,
+					int ramp_delay)
+{
+	int id = rdev_get_id(rdev);
+	unsigned int ramp_value;
+
+	if (id > MAX77802_BUCK4) {
+			dev_warn(&rdev->dev,
+				 "%s: regulator: ramp delay not supported\n",
+				 rdev->desc->name);
+		return -EINVAL;
+	}
+	ramp_value = max77802_find_ramp_value(rdev, ramp_table_77802_2bit,
+				ARRAY_SIZE(ramp_table_77802_2bit), ramp_delay);
+
+	return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
+				  MAX77802_RAMP_RATE_MASK_2BIT,
+				  ramp_value << MAX77802_RAMP_RATE_SHIFT_2BIT);
+}
+
+/* For BUCK1, 6 */
+static int max77802_set_ramp_delay_4bit(struct regulator_dev *rdev,
+					    int ramp_delay)
+{
+	unsigned int ramp_value;
+
+	ramp_value = max77802_find_ramp_value(rdev, ramp_table_77802_4bit,
+				ARRAY_SIZE(ramp_table_77802_4bit), ramp_delay);
+
+	return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
+				  MAX77802_RAMP_RATE_MASK_4BIT,
+				  ramp_value << MAX77802_RAMP_RATE_SHIFT_4BIT);
+}
+
+/*
+ * LDOs 2, 4-19, 22-35
+ */
+static struct regulator_ops max77802_ldo_ops_logic1 = {
+	.list_voltage		= regulator_list_voltage_linear,
+	.map_voltage		= regulator_map_voltage_linear,
+	.is_enabled		= regulator_is_enabled_regmap,
+	.enable			= max77802_enable,
+	.disable		= regulator_disable_regmap,
+	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
+	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
+	.set_voltage_time_sel	= regulator_set_voltage_time_sel,
+	.set_suspend_mode	= max77802_ldo_set_suspend_mode_logic1,
+};
+
+/*
+ * LDOs 1, 20, 21, 3
+ */
+static struct regulator_ops max77802_ldo_ops_logic2 = {
+	.list_voltage		= regulator_list_voltage_linear,
+	.map_voltage		= regulator_map_voltage_linear,
+	.is_enabled		= regulator_is_enabled_regmap,
+	.enable			= max77802_enable,
+	.disable		= regulator_disable_regmap,
+	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
+	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
+	.set_voltage_time_sel	= regulator_set_voltage_time_sel,
+	.set_suspend_mode	= max77802_ldo_set_suspend_mode_logic2,
+};
+
+/* BUCKS 1, 6 */
+static struct regulator_ops max77802_buck_16_dvs_ops = {
+	.list_voltage		= regulator_list_voltage_linear,
+	.map_voltage		= regulator_map_voltage_linear,
+	.is_enabled		= regulator_is_enabled_regmap,
+	.enable			= max77802_enable,
+	.disable		= regulator_disable_regmap,
+	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
+	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
+	.set_voltage_time_sel	= regulator_set_voltage_time_sel,
+	.set_ramp_delay		= max77802_set_ramp_delay_4bit,
+	.set_suspend_disable	= max77802_buck_set_suspend_disable,
+};
+
+/* BUCKs 2-4, 5, 7-10 */
+static struct regulator_ops max77802_buck_dvs_ops = {
+	.list_voltage		= regulator_list_voltage_linear,
+	.map_voltage		= regulator_map_voltage_linear,
+	.is_enabled		= regulator_is_enabled_regmap,
+	.enable			= max77802_enable,
+	.disable		= regulator_disable_regmap,
+	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
+	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
+	.set_voltage_time_sel	= regulator_set_voltage_time_sel,
+	.set_ramp_delay		= max77802_set_ramp_delay_2bit,
+	.set_suspend_disable	= max77802_buck_set_suspend_disable,
+};
+
+/* LDOs 3-7, 9-14, 18-26, 28, 29, 32-34 */
+#define regulator_77802_desc_p_ldo(num, supply, log)	{		\
+	.name		= "LDO"#num,					\
+	.id		= MAX77802_LDO##num,				\
+	.supply_name	= "inl"#supply,					\
+	.ops		= &max77802_ldo_ops_logic##log,			\
+	.type		= REGULATOR_VOLTAGE,				\
+	.owner		= THIS_MODULE,					\
+	.min_uV		= 800000,					\
+	.uV_step	= 50000,					\
+	.ramp_delay	= MAX77802_RAMP_DELAY,				\
+	.n_voltages	= 1 << 6,					\
+	.vsel_reg	= MAX77802_REG_LDO1CTRL1 + num - 1,		\
+	.vsel_mask	= MAX77802_VSEL_MASK,				\
+	.enable_reg	= MAX77802_REG_LDO1CTRL1 + num - 1,		\
+	.enable_mask	= MAX77802_OPMODE_MASK << MAX77802_OPMODE_SHIFT_LDO, \
+}
+
+/* LDOs 1, 2, 8, 15, 17, 27, 30, 35 */
+#define regulator_77802_desc_n_ldo(num, supply, log)   {		\
+	.name		= "LDO"#num,					\
+	.id		= MAX77802_LDO##num,				\
+	.supply_name	= "inl"#supply,					\
+	.ops		= &max77802_ldo_ops_logic##log,			\
+	.type		= REGULATOR_VOLTAGE,				\
+	.owner		= THIS_MODULE,					\
+	.min_uV		= 800000,					\
+	.uV_step	= 25000,					\
+	.ramp_delay	= MAX77802_RAMP_DELAY,				\
+	.n_voltages	= 1 << 6,					\
+	.vsel_reg	= MAX77802_REG_LDO1CTRL1 + num - 1,		\
+	.vsel_mask	= MAX77802_VSEL_MASK,				\
+	.enable_reg	= MAX77802_REG_LDO1CTRL1 + num - 1,		\
+	.enable_mask	= MAX77802_OPMODE_MASK << MAX77802_OPMODE_SHIFT_LDO, \
+}
+
+/* BUCKs 1, 6 */
+#define regulator_77802_desc_16_buck(num)	{		\
+	.name		= "BUCK"#num,					\
+	.id		= MAX77802_BUCK##num,				\
+	.supply_name	= "inb"#num,					\
+	.ops		= &max77802_buck_16_dvs_ops,			\
+	.type		= REGULATOR_VOLTAGE,				\
+	.owner		= THIS_MODULE,					\
+	.min_uV		= 612500,					\
+	.uV_step	= 6250,						\
+	.ramp_delay	= MAX77802_RAMP_DELAY,				\
+	.n_voltages	= 1 << 8,					\
+	.vsel_reg	= MAX77802_REG_BUCK ## num ## DVS1,		\
+	.vsel_mask	= MAX77802_DVS_VSEL_MASK,			\
+	.enable_reg	= MAX77802_REG_BUCK ## num ## CTRL,		\
+	.enable_mask	= MAX77802_OPMODE_MASK,				\
+}
+
+/* BUCKS 2-4 */
+#define regulator_77802_desc_234_buck(num)	{		\
+	.name		= "BUCK"#num,					\
+	.id		= MAX77802_BUCK##num,				\
+	.supply_name	= "inb"#num,					\
+	.ops		= &max77802_buck_dvs_ops,			\
+	.type		= REGULATOR_VOLTAGE,				\
+	.owner		= THIS_MODULE,					\
+	.min_uV		= 600000,					\
+	.uV_step	= 6250,						\
+	.ramp_delay	= MAX77802_RAMP_DELAY,				\
+	.n_voltages	= 0x91,						\
+	.vsel_reg	= MAX77802_REG_BUCK ## num ## DVS1,		\
+	.vsel_mask	= MAX77802_DVS_VSEL_MASK,			\
+	.enable_reg	= MAX77802_REG_BUCK ## num ## CTRL1,		\
+	.enable_mask	= MAX77802_OPMODE_MASK <<			\
+				MAX77802_OPMODE_BUCK234_SHIFT,		\
+}
+
+/* BUCK 5 */
+#define regulator_77802_desc_buck5(num)		{		\
+	.name		= "BUCK"#num,					\
+	.id		= MAX77802_BUCK##num,				\
+	.supply_name	= "inb"#num,					\
+	.ops		= &max77802_buck_dvs_ops,			\
+	.type		= REGULATOR_VOLTAGE,				\
+	.owner		= THIS_MODULE,					\
+	.min_uV		= 750000,					\
+	.uV_step	= 50000,					\
+	.ramp_delay	= MAX77802_RAMP_DELAY,				\
+	.n_voltages	= 1 << 6,					\
+	.vsel_reg	= MAX77802_REG_BUCK5OUT,			\
+	.vsel_mask	= MAX77802_VSEL_MASK,				\
+	.enable_reg	= MAX77802_REG_BUCK5CTRL,			\
+	.enable_mask	= MAX77802_OPMODE_MASK,				\
+}
+
+/* BUCKs 7-10 */
+#define regulator_77802_desc_buck7_10(num)	{		\
+	.name		= "BUCK"#num,					\
+	.id		= MAX77802_BUCK##num,				\
+	.supply_name	= "inb"#num,					\
+	.ops		= &max77802_buck_dvs_ops,			\
+	.type		= REGULATOR_VOLTAGE,				\
+	.owner		= THIS_MODULE,					\
+	.min_uV		= 750000,					\
+	.uV_step	= 50000,					\
+	.ramp_delay	= MAX77802_RAMP_DELAY,				\
+	.n_voltages	= 1 << 6,					\
+	.vsel_reg	= MAX77802_REG_BUCK7OUT + (num - 7) * 3,	\
+	.vsel_mask	= MAX77802_VSEL_MASK,				\
+	.enable_reg	= MAX77802_REG_BUCK7CTRL + (num - 7) * 3,	\
+	.enable_mask	= MAX77802_OPMODE_MASK,				\
+}
+
+static struct regulator_desc regulators[] = {
+	regulator_77802_desc_16_buck(1),
+	regulator_77802_desc_234_buck(2),
+	regulator_77802_desc_234_buck(3),
+	regulator_77802_desc_234_buck(4),
+	regulator_77802_desc_buck5(5),
+	regulator_77802_desc_16_buck(6),
+	regulator_77802_desc_buck7_10(7),
+	regulator_77802_desc_buck7_10(8),
+	regulator_77802_desc_buck7_10(9),
+	regulator_77802_desc_buck7_10(10),
+	regulator_77802_desc_n_ldo(1, 10, 2),
+	regulator_77802_desc_n_ldo(2, 10, 1),
+	regulator_77802_desc_p_ldo(3, 3, 2),
+	regulator_77802_desc_p_ldo(4, 6, 1),
+	regulator_77802_desc_p_ldo(5, 3, 1),
+	regulator_77802_desc_p_ldo(6, 3, 1),
+	regulator_77802_desc_p_ldo(7, 3, 1),
+	regulator_77802_desc_n_ldo(8, 1, 1),
+	regulator_77802_desc_p_ldo(9, 5, 1),
+	regulator_77802_desc_p_ldo(10, 4, 1),
+	regulator_77802_desc_p_ldo(11, 4, 1),
+	regulator_77802_desc_p_ldo(12, 9, 1),
+	regulator_77802_desc_p_ldo(13, 4, 1),
+	regulator_77802_desc_p_ldo(14, 4, 1),
+	regulator_77802_desc_n_ldo(15, 1, 1),
+	regulator_77802_desc_n_ldo(17, 2, 1),
+	regulator_77802_desc_p_ldo(18, 7, 1),
+	regulator_77802_desc_p_ldo(19, 5, 1),
+	regulator_77802_desc_p_ldo(20, 7, 2),
+	regulator_77802_desc_p_ldo(21, 6, 2),
+	regulator_77802_desc_p_ldo(23, 9, 1),
+	regulator_77802_desc_p_ldo(24, 6, 1),
+	regulator_77802_desc_p_ldo(25, 9, 1),
+	regulator_77802_desc_p_ldo(26, 9, 1),
+	regulator_77802_desc_n_ldo(27, 2, 1),
+	regulator_77802_desc_p_ldo(28, 7, 1),
+	regulator_77802_desc_p_ldo(29, 7, 1),
+	regulator_77802_desc_n_ldo(30, 2, 1),
+	regulator_77802_desc_p_ldo(32, 9, 1),
+	regulator_77802_desc_p_ldo(33, 6, 1),
+	regulator_77802_desc_p_ldo(34, 9, 1),
+	regulator_77802_desc_n_ldo(35, 2, 1),
+};
+
+#ifdef CONFIG_OF
+static int max77802_pmic_dt_parse_pdata(struct platform_device *pdev,
+					struct max77686_platform_data *pdata)
+{
+	struct max77686_dev *iodev = dev_get_drvdata(pdev->dev.parent);
+	struct device_node *pmic_np, *regulators_np;
+	struct max77686_regulator_data *rdata;
+	struct of_regulator_match rmatch;
+	unsigned int i;
+
+	pmic_np = iodev->dev->of_node;
+	regulators_np = of_get_child_by_name(pmic_np, "regulators");
+	if (!regulators_np) {
+		dev_err(&pdev->dev, "could not find regulators sub-node\n");
+		return -EINVAL;
+	}
+
+	pdata->num_regulators = ARRAY_SIZE(regulators);
+	rdata = devm_kzalloc(&pdev->dev, sizeof(*rdata) *
+			     pdata->num_regulators, GFP_KERNEL);
+	if (!rdata) {
+		of_node_put(regulators_np);
+		return -ENOMEM;
+	}
+
+	for (i = 0; i < pdata->num_regulators; i++) {
+		rmatch.name = regulators[i].name;
+		rmatch.init_data = NULL;
+		rmatch.of_node = NULL;
+		if (of_regulator_match(&pdev->dev, regulators_np, &rmatch,
+				       1) != 1) {
+			dev_warn(&pdev->dev, "No matching regulator for '%s'\n",
+				 rmatch.name);
+			continue;
+		}
+		rdata[i].initdata = rmatch.init_data;
+		rdata[i].of_node = rmatch.of_node;
+		rdata[i].id = regulators[i].id;
+	}
+
+	pdata->regulators = rdata;
+	of_node_put(regulators_np);
+
+	return 0;
+}
+#else
+static int max77802_pmic_dt_parse_pdata(struct platform_device *pdev,
+					struct max77686_platform_data *pdata)
+{
+	return 0;
+}
+#endif /* CONFIG_OF */
+
+static int max77802_pmic_probe(struct platform_device *pdev)
+{
+	struct max77686_dev *iodev = dev_get_drvdata(pdev->dev.parent);
+	struct max77686_platform_data *pdata = dev_get_platdata(iodev->dev);
+	struct max77802_regulator_prv *max77802;
+	int i, ret = 0, val;
+	struct regulator_config config = { };
+
+	/* This is allocated by the MFD driver */
+	if (!pdata) {
+		dev_err(&pdev->dev, "no platform data found for regulator\n");
+		return -ENODEV;
+	}
+
+	max77802 = devm_kzalloc(&pdev->dev,
+				sizeof(struct max77802_regulator_prv),
+				GFP_KERNEL);
+	if (!max77802)
+		return -ENOMEM;
+
+	if (iodev->dev->of_node) {
+		ret = max77802_pmic_dt_parse_pdata(pdev, pdata);
+		if (ret)
+			return ret;
+	}
+
+	config.dev = iodev->dev;
+	config.regmap = iodev->regmap;
+	config.driver_data = max77802;
+	platform_set_drvdata(pdev, max77802);
+
+	for (i = 0; i < MAX77802_REG_MAX; i++) {
+		struct regulator_dev *rdev;
+		int id = pdata->regulators[i].id;
+		int shift = max77802_get_opmode_shift(id);
+
+		config.init_data = pdata->regulators[i].initdata;
+		config.of_node = pdata->regulators[i].of_node;
+
+		ret = regmap_read(iodev->regmap, regulators[i].enable_reg, &val);
+		max77802->opmode[id] = val >> shift & MAX77802_OPMODE_MASK;
+
+		rdev = devm_regulator_register(&pdev->dev,
+					       &regulators[i], &config);
+		if (IS_ERR(rdev)) {
+			dev_err(&pdev->dev,
+				"regulator init failed for %d\n", i);
+			return PTR_ERR(rdev);
+		}
+	}
+
+	return 0;
+}
+
+static const struct platform_device_id max77802_pmic_id[] = {
+	{"max77802-pmic", 0},
+	{ },
+};
+MODULE_DEVICE_TABLE(platform, max77802_pmic_id);
+
+static struct platform_driver max77802_pmic_driver = {
+	.driver = {
+		.name = "max77802-pmic",
+		.owner = THIS_MODULE,
+	},
+	.probe = max77802_pmic_probe,
+	.id_table = max77802_pmic_id,
+};
+
+module_platform_driver(max77802_pmic_driver);
+
+MODULE_DESCRIPTION("MAXIM 77802 Regulator Driver");
+MODULE_AUTHOR("Simon Glass <sjg@chromium.org>");
+MODULE_LICENSE("GPL");
-- 
2.0.1

[PATCH v9 2/2] regulator: Add DT bindings for max77802 PMIC regulators

[Javier Martinez Canillas]

Add Device Tree binding documentation for the regulators
present in the Maxim 77802 Power Management IC.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 .../devicetree/bindings/regulator/max77802.txt     | 53 ++++++++++++++++++++++
 1 file changed, 53 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/max77802.txt

diff --git a/Documentation/devicetree/bindings/regulator/max77802.txt b/Documentation/devicetree/bindings/regulator/max77802.txt
new file mode 100644
index 0000000..5aeaffc
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/max77802.txt
@@ -0,0 +1,53 @@
+Binding for Maxim MAX77802 regulators
+
+This is a part of device tree bindings of MAX77802 multi-function device.
+More information can be found in bindings/mfd/max77802.txt file.
+
+The MAX77802 PMIC has 10 high-efficiency Buck and 32 Low-dropout (LDO)
+regulators that can be controlled over I2C.
+
+Following properties should be present in main device node of the MFD chip.
+
+Optional node:
+- regulators : The regulators of max77802 have to be instantiated
+  under subnode named "regulators" using the following format.
+
+	regulator-name {
+		standard regulator constraints....
+	};
+	refer Documentation/devicetree/bindings/regulator/regulator.txt
+
+The regulator node name should be initialized with a string to get matched
+with their hardware counterparts as follow. The valid names are:
+
+	-LDOn 	:	for LDOs, where n can lie in ranges 1-15, 17-21, 23-30
+			and 32-35.
+			example: LDO1, LDO2, LDO35.
+	-BUCKn 	:	for BUCKs, where n can lie in range 1 to 10.
+			example: BUCK1, BUCK5, BUCK10.
+Example:
+
+	max77802@09 {
+		compatible = "maxim,max77802";
+		interrupt-parent = <&wakeup_eint>;
+		interrupts = <26 0>;
+		reg = <0x09>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		regulators {
+			ldo11_reg: LDO11 {
+				regulator-name = "vdd_ldo11";
+				regulator-min-microvolt = <1900000>;
+				regulator-max-microvolt = <1900000>;
+				regulator-always-on;
+			};
+
+			buck1_reg: BUCK1 {
+				regulator-name = "vdd_mif";
+				regulator-min-microvolt = <950000>;
+				regulator-max-microvolt = <1300000>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+	};
-- 
2.0.1

Re: [PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

[Mark Brown]

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

On Mon, Aug 18, 2014 at 10:32:41AM +0200, Javier Martinez Canillas wrote:
> The MAX77802 PMIC has 10 high-efficiency Buck and 32 Low-dropout
> (LDO) regulators. This patch adds support for all these regulators
> found on the MAX77802 PMIC and is based on a driver added by Simon
> Glass to the Chrome OS kernel 3.8 tree.

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v9 2/2] regulator: Add DT bindings for max77802 PMIC regulators

[Mark Brown]

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

On Mon, Aug 18, 2014 at 10:32:42AM +0200, Javier Martinez Canillas wrote:
> Add Device Tree binding documentation for the regulators
> present in the Maxim 77802 Power Management IC.

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

[Yuvaraj Cd]

On Mon, Aug 18, 2014 at 2:02 PM, Javier Martinez Canillas
<javier.martinez@collabora.co.uk> wrote:
> The MAX77802 PMIC has 10 high-efficiency Buck and 32 Low-dropout
> (LDO) regulators. This patch adds support for all these regulators
> found on the MAX77802 PMIC and is based on a driver added by Simon
> Glass to the Chrome OS kernel 3.8 tree.
>
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> Tested-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
> ---
>
> Changes since v7:
>  - Remove DVS support since that can be added as a follow up.
>
> Changes since v6: None
>
> Changes since v5:
>  - Take out the mfd changes from v4 that were squashed by mistake.
>    Suggested by Lee Jones.
>
> Changes since v4: None
>
> Changes since v3:
>  - Set the supply_name for regulators to lookup their parent supply node.
>    Suggested by Mark Brown.
>  - Change Exyno5 for Exynos5420/Exynos5800 in regulator driver Kconfig.
>    Suggested by Doug Anderson.
> ---
>  drivers/regulator/Kconfig    |   9 +
>  drivers/regulator/Makefile   |   1 +
>  drivers/regulator/max77802.c | 578 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 588 insertions(+)
>  create mode 100644 drivers/regulator/max77802.c
>
> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> index 2dc8289..8134a99 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -387,6 +387,15 @@ config REGULATOR_MAX77693
>           and one current regulator 'CHARGER'. This is suitable for
>           Exynos-4x12 chips.
>
> +config REGULATOR_MAX77802
> +       tristate "Maxim 77802 regulator"
> +       depends on MFD_MAX77686
> +       help
> +         This driver controls a Maxim 77802 regulator
> +         via I2C bus. The provided regulator is suitable for
> +         Exynos5420/Exynos5800 SoCs to control various voltages.
> +         It includes support for control of voltage and ramp speed.
> +
>  config REGULATOR_MC13XXX_CORE
>         tristate
>
> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
> index aa4a6aa..b4ec6c8 100644
> --- a/drivers/regulator/Makefile
> +++ b/drivers/regulator/Makefile
> @@ -52,6 +52,7 @@ obj-$(CONFIG_REGULATOR_MAX8997) += max8997.o
>  obj-$(CONFIG_REGULATOR_MAX8998) += max8998.o
>  obj-$(CONFIG_REGULATOR_MAX77686) += max77686.o
>  obj-$(CONFIG_REGULATOR_MAX77693) += max77693.o
> +obj-$(CONFIG_REGULATOR_MAX77802) += max77802.o
>  obj-$(CONFIG_REGULATOR_MC13783) += mc13783-regulator.o
>  obj-$(CONFIG_REGULATOR_MC13892) += mc13892-regulator.o
>  obj-$(CONFIG_REGULATOR_MC13XXX_CORE) +=  mc13xxx-regulator-core.o
> diff --git a/drivers/regulator/max77802.c b/drivers/regulator/max77802.c
> new file mode 100644
> index 0000000..5f022f8
> --- /dev/null
> +++ b/drivers/regulator/max77802.c
> @@ -0,0 +1,578 @@
> +/*
> + * max77802.c - Regulator driver for the Maxim 77802
> + *
> + * Copyright (C) 2013-2014 Google, Inc
> + * Simon Glass <sjg@chromium.org>
> + *
> + * Copyright (C) 2012 Samsung Electronics
> + * Chiwoong Byun <woong.byun@smasung.com>
> + * Jonghwa Lee <jonghwa3.lee@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * 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.
> + *
> + * This driver is based on max8997.c
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/bug.h>
> +#include <linux/err.h>
> +#include <linux/gpio.h>
> +#include <linux/slab.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/regulator/machine.h>
> +#include <linux/regulator/of_regulator.h>
> +#include <linux/mfd/max77686.h>
> +#include <linux/mfd/max77686-private.h>
> +
> +/* Default ramp delay in case it is not manually set */
> +#define MAX77802_RAMP_DELAY            100000          /* uV/us */
> +
> +#define MAX77802_OPMODE_SHIFT_LDO      6
> +#define MAX77802_OPMODE_BUCK234_SHIFT  4
> +#define MAX77802_OPMODE_MASK           0x3
> +
> +#define MAX77802_VSEL_MASK             0x3F
> +#define MAX77802_DVS_VSEL_MASK         0xFF
> +
> +#define MAX77802_RAMP_RATE_MASK_2BIT   0xC0
> +#define MAX77802_RAMP_RATE_SHIFT_2BIT  6
> +#define MAX77802_RAMP_RATE_MASK_4BIT   0xF0
> +#define MAX77802_RAMP_RATE_SHIFT_4BIT  4
> +
> +/* MAX77802 has two register formats: 2-bit and 4-bit */
> +static const unsigned int ramp_table_77802_2bit[] = {
> +       12500,
> +       25000,
> +       50000,
> +       100000,
> +};
> +
> +static unsigned int ramp_table_77802_4bit[] = {
> +       1000,   2000,   3030,   4000,
> +       5000,   5880,   7140,   8330,
> +       9090,   10000,  11110,  12500,
> +       16670,  25000,  50000,  100000,
> +};
> +
> +struct max77802_regulator_prv {
> +       int num_regulators;
> +       struct regulator_dev *rdev[MAX77802_REG_MAX];
> +       unsigned int opmode[MAX77802_REG_MAX];
> +};
> +
> +static int max77802_get_opmode_shift(int id)
> +{
> +       if (id == MAX77802_BUCK1 || (id >= MAX77802_BUCK5 &&
> +                                    id <= MAX77802_BUCK10))
> +               return 0;
> +
> +       if (id >= MAX77802_BUCK2 && id <= MAX77802_BUCK4)
> +               return MAX77802_OPMODE_BUCK234_SHIFT;
> +
> +       if (id >= MAX77802_LDO1 && id <= MAX77802_LDO35)
> +               return MAX77802_OPMODE_SHIFT_LDO;
> +
> +       return -EINVAL;
> +}
> +
> +/*
> + * Some BUCKS supports Normal[ON/OFF] mode during suspend
> + *
> + * BUCK 1, 6, 2-4, 5, 7-10 (all)
> + *
> + * The other mode (0x02) will make PWRREQ switch between normal
> + * and low power.
> + */
> +static int max77802_buck_set_suspend_disable(struct regulator_dev *rdev)
> +{
> +       unsigned int val = MAX77802_OPMODE_STANDBY;
> +       struct max77802_regulator_prv *max77802 = rdev_get_drvdata(rdev);
> +       int id = rdev_get_id(rdev);
> +       int shift = max77802_get_opmode_shift(id);
> +
> +       max77802->opmode[id] = val;
> +       return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
> +                                 rdev->desc->enable_mask, val << shift);
> +}
> +
> +/*
> + * Some LDOs supports LPM-ON/OFF/Normal-ON mode during suspend state
> + * (Enable Control Logic1 by PWRREQ)
> + *
> + * LDOs 2, 4-19, 22-35.
> + *
> + */
> +static int max77802_ldo_set_suspend_mode_logic1(struct regulator_dev *rdev,
> +                                               unsigned int mode)
> +{
> +       struct max77802_regulator_prv *max77802 = rdev_get_drvdata(rdev);
> +       int id = rdev_get_id(rdev);
> +       unsigned int val;
> +       int shift = max77802_get_opmode_shift(id);
> +
> +       switch (mode) {
> +       case REGULATOR_MODE_IDLE:                       /* ON in LP Mode */
> +               val = MAX77802_OPMODE_LP;
> +               break;
> +       case REGULATOR_MODE_NORMAL:                     /* ON in Normal Mode */
> +               val = MAX77802_OPMODE_NORMAL;
> +               break;
> +       case REGULATOR_MODE_STANDBY:                    /* ON/OFF by PWRREQ */
> +               val = MAX77802_OPMODE_STANDBY;
> +               break;
> +       default:
> +               dev_warn(&rdev->dev, "%s: regulator mode: 0x%x not supported\n",
> +                        rdev->desc->name, mode);
> +               return -EINVAL;
> +       }
> +
> +       max77802->opmode[rdev_get_id(rdev)] = val;
> +       return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
> +                                 rdev->desc->enable_mask, val << shift);
> +}
> +
> +/*
> + * Mode 1 (Output[ON/OFF] by PWRREQ) is not supported on some LDOs
> + * (Enable Control Logic2 by PWRREQ)
> + *
> + * LDOs 1, 20, 21, and 3,
> + *
> + */
> +static int max77802_ldo_set_suspend_mode_logic2(struct regulator_dev *rdev,
> +                                               unsigned int mode)
> +{
> +       struct max77802_regulator_prv *max77802 = rdev_get_drvdata(rdev);
> +       int id = rdev_get_id(rdev);
> +       unsigned int val;
> +       int shift = max77802_get_opmode_shift(id);
> +
> +       switch (mode) {
> +       case REGULATOR_MODE_IDLE:                       /* ON in LP Mode */
> +               val = MAX77802_OPMODE_LP;
> +               break;
> +       case REGULATOR_MODE_NORMAL:                     /* ON in Normal Mode */
> +               val = MAX77802_OPMODE_NORMAL;
> +               break;
> +       default:
> +               dev_warn(&rdev->dev, "%s: regulator mode: 0x%x not supported\n",
> +                        rdev->desc->name, mode);
> +               return -EINVAL;
> +       }
> +
> +       max77802->opmode[rdev_get_id(rdev)] = val;
> +       return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
> +                                 rdev->desc->enable_mask, val << shift);
> +}
> +
> +static int max77802_enable(struct regulator_dev *rdev)
> +{
> +       struct max77802_regulator_prv *max77802 = rdev_get_drvdata(rdev);
> +       int id = rdev_get_id(rdev);
> +       int shift = max77802_get_opmode_shift(id);
> +
> +       return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
> +                                 rdev->desc->enable_mask,
> +                                 max77802->opmode[id] << shift);
> +}
> +
> +static int max77802_find_ramp_value(struct regulator_dev *rdev,
> +                                   const unsigned int limits[], int size,
> +                                   unsigned int ramp_delay)
> +{
> +       int i;
> +
> +       for (i = 0; i < size; i++) {
> +               if (ramp_delay <= limits[i])
> +                       return i;
> +       }
> +
> +       /* Use maximum value for no ramp control */
> +       dev_warn(&rdev->dev, "%s: ramp_delay: %d not supported, setting 100000\n",
> +                rdev->desc->name, ramp_delay);
> +       return size - 1;
> +}
> +
> +/* Used for BUCKs 2-4 */
> +static int max77802_set_ramp_delay_2bit(struct regulator_dev *rdev,
> +                                       int ramp_delay)
> +{
> +       int id = rdev_get_id(rdev);
> +       unsigned int ramp_value;
> +
> +       if (id > MAX77802_BUCK4) {
> +                       dev_warn(&rdev->dev,
> +                                "%s: regulator: ramp delay not supported\n",
> +                                rdev->desc->name);
> +               return -EINVAL;
> +       }
> +       ramp_value = max77802_find_ramp_value(rdev, ramp_table_77802_2bit,
> +                               ARRAY_SIZE(ramp_table_77802_2bit), ramp_delay);
> +
> +       return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
> +                                 MAX77802_RAMP_RATE_MASK_2BIT,
> +                                 ramp_value << MAX77802_RAMP_RATE_SHIFT_2BIT);
> +}
> +
> +/* For BUCK1, 6 */
> +static int max77802_set_ramp_delay_4bit(struct regulator_dev *rdev,
> +                                           int ramp_delay)
> +{
> +       unsigned int ramp_value;
> +
> +       ramp_value = max77802_find_ramp_value(rdev, ramp_table_77802_4bit,
> +                               ARRAY_SIZE(ramp_table_77802_4bit), ramp_delay);
> +
> +       return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
> +                                 MAX77802_RAMP_RATE_MASK_4BIT,
> +                                 ramp_value << MAX77802_RAMP_RATE_SHIFT_4BIT);
> +}
> +
> +/*
> + * LDOs 2, 4-19, 22-35
> + */
> +static struct regulator_ops max77802_ldo_ops_logic1 = {
> +       .list_voltage           = regulator_list_voltage_linear,
> +       .map_voltage            = regulator_map_voltage_linear,
> +       .is_enabled             = regulator_is_enabled_regmap,
> +       .enable                 = max77802_enable,
> +       .disable                = regulator_disable_regmap,
> +       .get_voltage_sel        = regulator_get_voltage_sel_regmap,
> +       .set_voltage_sel        = regulator_set_voltage_sel_regmap,
> +       .set_voltage_time_sel   = regulator_set_voltage_time_sel,
> +       .set_suspend_mode       = max77802_ldo_set_suspend_mode_logic1,
> +};
> +
> +/*
> + * LDOs 1, 20, 21, 3
> + */
> +static struct regulator_ops max77802_ldo_ops_logic2 = {
> +       .list_voltage           = regulator_list_voltage_linear,
> +       .map_voltage            = regulator_map_voltage_linear,
> +       .is_enabled             = regulator_is_enabled_regmap,
> +       .enable                 = max77802_enable,
> +       .disable                = regulator_disable_regmap,
> +       .get_voltage_sel        = regulator_get_voltage_sel_regmap,
> +       .set_voltage_sel        = regulator_set_voltage_sel_regmap,
> +       .set_voltage_time_sel   = regulator_set_voltage_time_sel,
> +       .set_suspend_mode       = max77802_ldo_set_suspend_mode_logic2,
> +};
> +
> +/* BUCKS 1, 6 */
> +static struct regulator_ops max77802_buck_16_dvs_ops = {
> +       .list_voltage           = regulator_list_voltage_linear,
> +       .map_voltage            = regulator_map_voltage_linear,
> +       .is_enabled             = regulator_is_enabled_regmap,
> +       .enable                 = max77802_enable,
> +       .disable                = regulator_disable_regmap,
> +       .get_voltage_sel        = regulator_get_voltage_sel_regmap,
> +       .set_voltage_sel        = regulator_set_voltage_sel_regmap,
> +       .set_voltage_time_sel   = regulator_set_voltage_time_sel,
> +       .set_ramp_delay         = max77802_set_ramp_delay_4bit,
> +       .set_suspend_disable    = max77802_buck_set_suspend_disable,
> +};
> +
> +/* BUCKs 2-4, 5, 7-10 */
> +static struct regulator_ops max77802_buck_dvs_ops = {
> +       .list_voltage           = regulator_list_voltage_linear,
> +       .map_voltage            = regulator_map_voltage_linear,
> +       .is_enabled             = regulator_is_enabled_regmap,
> +       .enable                 = max77802_enable,
> +       .disable                = regulator_disable_regmap,
> +       .get_voltage_sel        = regulator_get_voltage_sel_regmap,
> +       .set_voltage_sel        = regulator_set_voltage_sel_regmap,
> +       .set_voltage_time_sel   = regulator_set_voltage_time_sel,
> +       .set_ramp_delay         = max77802_set_ramp_delay_2bit,
> +       .set_suspend_disable    = max77802_buck_set_suspend_disable,
> +};
> +
> +/* LDOs 3-7, 9-14, 18-26, 28, 29, 32-34 */
> +#define regulator_77802_desc_p_ldo(num, supply, log)   {               \
> +       .name           = "LDO"#num,                                    \
> +       .id             = MAX77802_LDO##num,                            \
> +       .supply_name    = "inl"#supply,                                 \
> +       .ops            = &max77802_ldo_ops_logic##log,                 \
> +       .type           = REGULATOR_VOLTAGE,                            \
> +       .owner          = THIS_MODULE,                                  \
> +       .min_uV         = 800000,                                       \
> +       .uV_step        = 50000,                                        \
> +       .ramp_delay     = MAX77802_RAMP_DELAY,                          \
> +       .n_voltages     = 1 << 6,                                       \
> +       .vsel_reg       = MAX77802_REG_LDO1CTRL1 + num - 1,             \
> +       .vsel_mask      = MAX77802_VSEL_MASK,                           \
> +       .enable_reg     = MAX77802_REG_LDO1CTRL1 + num - 1,             \
> +       .enable_mask    = MAX77802_OPMODE_MASK << MAX77802_OPMODE_SHIFT_LDO, \
> +}
> +
> +/* LDOs 1, 2, 8, 15, 17, 27, 30, 35 */
> +#define regulator_77802_desc_n_ldo(num, supply, log)   {               \
> +       .name           = "LDO"#num,                                    \
> +       .id             = MAX77802_LDO##num,                            \
> +       .supply_name    = "inl"#supply,                                 \
> +       .ops            = &max77802_ldo_ops_logic##log,                 \
> +       .type           = REGULATOR_VOLTAGE,                            \
> +       .owner          = THIS_MODULE,                                  \
> +       .min_uV         = 800000,                                       \
> +       .uV_step        = 25000,                                        \
> +       .ramp_delay     = MAX77802_RAMP_DELAY,                          \
> +       .n_voltages     = 1 << 6,                                       \
> +       .vsel_reg       = MAX77802_REG_LDO1CTRL1 + num - 1,             \
> +       .vsel_mask      = MAX77802_VSEL_MASK,                           \
> +       .enable_reg     = MAX77802_REG_LDO1CTRL1 + num - 1,             \
> +       .enable_mask    = MAX77802_OPMODE_MASK << MAX77802_OPMODE_SHIFT_LDO, \
> +}
> +
> +/* BUCKs 1, 6 */
> +#define regulator_77802_desc_16_buck(num)      {               \
> +       .name           = "BUCK"#num,                                   \
> +       .id             = MAX77802_BUCK##num,                           \
> +       .supply_name    = "inb"#num,                                    \
> +       .ops            = &max77802_buck_16_dvs_ops,                    \
> +       .type           = REGULATOR_VOLTAGE,                            \
> +       .owner          = THIS_MODULE,                                  \
> +       .min_uV         = 612500,                                       \
> +       .uV_step        = 6250,                                         \
> +       .ramp_delay     = MAX77802_RAMP_DELAY,                          \
> +       .n_voltages     = 1 << 8,                                       \
> +       .vsel_reg       = MAX77802_REG_BUCK ## num ## DVS1,             \
> +       .vsel_mask      = MAX77802_DVS_VSEL_MASK,                       \
> +       .enable_reg     = MAX77802_REG_BUCK ## num ## CTRL,             \
> +       .enable_mask    = MAX77802_OPMODE_MASK,                         \
> +}
> +
> +/* BUCKS 2-4 */
> +#define regulator_77802_desc_234_buck(num)     {               \
> +       .name           = "BUCK"#num,                                   \
> +       .id             = MAX77802_BUCK##num,                           \
> +       .supply_name    = "inb"#num,                                    \
> +       .ops            = &max77802_buck_dvs_ops,                       \
> +       .type           = REGULATOR_VOLTAGE,                            \
> +       .owner          = THIS_MODULE,                                  \
> +       .min_uV         = 600000,                                       \
> +       .uV_step        = 6250,                                         \
> +       .ramp_delay     = MAX77802_RAMP_DELAY,                          \
> +       .n_voltages     = 0x91,                                         \
> +       .vsel_reg       = MAX77802_REG_BUCK ## num ## DVS1,             \
> +       .vsel_mask      = MAX77802_DVS_VSEL_MASK,                       \
> +       .enable_reg     = MAX77802_REG_BUCK ## num ## CTRL1,            \
> +       .enable_mask    = MAX77802_OPMODE_MASK <<                       \
> +                               MAX77802_OPMODE_BUCK234_SHIFT,          \
> +}
> +
> +/* BUCK 5 */
> +#define regulator_77802_desc_buck5(num)                {               \
> +       .name           = "BUCK"#num,                                   \
> +       .id             = MAX77802_BUCK##num,                           \
> +       .supply_name    = "inb"#num,                                    \
> +       .ops            = &max77802_buck_dvs_ops,                       \
> +       .type           = REGULATOR_VOLTAGE,                            \
> +       .owner          = THIS_MODULE,                                  \
> +       .min_uV         = 750000,                                       \
> +       .uV_step        = 50000,                                        \
> +       .ramp_delay     = MAX77802_RAMP_DELAY,                          \
> +       .n_voltages     = 1 << 6,                                       \
> +       .vsel_reg       = MAX77802_REG_BUCK5OUT,                        \
> +       .vsel_mask      = MAX77802_VSEL_MASK,                           \
> +       .enable_reg     = MAX77802_REG_BUCK5CTRL,                       \
> +       .enable_mask    = MAX77802_OPMODE_MASK,                         \
> +}
> +
> +/* BUCKs 7-10 */
> +#define regulator_77802_desc_buck7_10(num)     {               \
> +       .name           = "BUCK"#num,                                   \
> +       .id             = MAX77802_BUCK##num,                           \
> +       .supply_name    = "inb"#num,                                    \
> +       .ops            = &max77802_buck_dvs_ops,                       \
> +       .type           = REGULATOR_VOLTAGE,                            \
> +       .owner          = THIS_MODULE,                                  \
> +       .min_uV         = 750000,                                       \
> +       .uV_step        = 50000,                                        \
> +       .ramp_delay     = MAX77802_RAMP_DELAY,                          \
> +       .n_voltages     = 1 << 6,                                       \
> +       .vsel_reg       = MAX77802_REG_BUCK7OUT + (num - 7) * 3,        \
> +       .vsel_mask      = MAX77802_VSEL_MASK,                           \
> +       .enable_reg     = MAX77802_REG_BUCK7CTRL + (num - 7) * 3,       \
> +       .enable_mask    = MAX77802_OPMODE_MASK,                         \
> +}
> +
> +static struct regulator_desc regulators[] = {
> +       regulator_77802_desc_16_buck(1),
> +       regulator_77802_desc_234_buck(2),
> +       regulator_77802_desc_234_buck(3),
> +       regulator_77802_desc_234_buck(4),
> +       regulator_77802_desc_buck5(5),
> +       regulator_77802_desc_16_buck(6),
> +       regulator_77802_desc_buck7_10(7),
> +       regulator_77802_desc_buck7_10(8),
> +       regulator_77802_desc_buck7_10(9),
> +       regulator_77802_desc_buck7_10(10),
> +       regulator_77802_desc_n_ldo(1, 10, 2),
> +       regulator_77802_desc_n_ldo(2, 10, 1),
> +       regulator_77802_desc_p_ldo(3, 3, 2),
> +       regulator_77802_desc_p_ldo(4, 6, 1),
> +       regulator_77802_desc_p_ldo(5, 3, 1),
> +       regulator_77802_desc_p_ldo(6, 3, 1),
> +       regulator_77802_desc_p_ldo(7, 3, 1),
> +       regulator_77802_desc_n_ldo(8, 1, 1),
> +       regulator_77802_desc_p_ldo(9, 5, 1),
> +       regulator_77802_desc_p_ldo(10, 4, 1),
> +       regulator_77802_desc_p_ldo(11, 4, 1),
> +       regulator_77802_desc_p_ldo(12, 9, 1),
> +       regulator_77802_desc_p_ldo(13, 4, 1),
> +       regulator_77802_desc_p_ldo(14, 4, 1),
> +       regulator_77802_desc_n_ldo(15, 1, 1),
> +       regulator_77802_desc_n_ldo(17, 2, 1),
> +       regulator_77802_desc_p_ldo(18, 7, 1),
> +       regulator_77802_desc_p_ldo(19, 5, 1),
> +       regulator_77802_desc_p_ldo(20, 7, 2),
> +       regulator_77802_desc_p_ldo(21, 6, 2),
> +       regulator_77802_desc_p_ldo(23, 9, 1),
> +       regulator_77802_desc_p_ldo(24, 6, 1),
> +       regulator_77802_desc_p_ldo(25, 9, 1),
> +       regulator_77802_desc_p_ldo(26, 9, 1),
> +       regulator_77802_desc_n_ldo(27, 2, 1),
> +       regulator_77802_desc_p_ldo(28, 7, 1),
> +       regulator_77802_desc_p_ldo(29, 7, 1),
> +       regulator_77802_desc_n_ldo(30, 2, 1),
> +       regulator_77802_desc_p_ldo(32, 9, 1),
> +       regulator_77802_desc_p_ldo(33, 6, 1),
> +       regulator_77802_desc_p_ldo(34, 9, 1),
> +       regulator_77802_desc_n_ldo(35, 2, 1),
> +};
> +
> +#ifdef CONFIG_OF
> +static int max77802_pmic_dt_parse_pdata(struct platform_device *pdev,
> +                                       struct max77686_platform_data *pdata)
> +{
> +       struct max77686_dev *iodev = dev_get_drvdata(pdev->dev.parent);
> +       struct device_node *pmic_np, *regulators_np;
> +       struct max77686_regulator_data *rdata;
> +       struct of_regulator_match rmatch;
> +       unsigned int i;
> +
> +       pmic_np = iodev->dev->of_node;
> +       regulators_np = of_get_child_by_name(pmic_np, "regulators");
> +       if (!regulators_np) {
> +               dev_err(&pdev->dev, "could not find regulators sub-node\n");
> +               return -EINVAL;
> +       }
> +
> +       pdata->num_regulators = ARRAY_SIZE(regulators);
> +       rdata = devm_kzalloc(&pdev->dev, sizeof(*rdata) *
> +                            pdata->num_regulators, GFP_KERNEL);
> +       if (!rdata) {
> +               of_node_put(regulators_np);
> +               return -ENOMEM;
> +       }
> +
> +       for (i = 0; i < pdata->num_regulators; i++) {
> +               rmatch.name = regulators[i].name;
> +               rmatch.init_data = NULL;
> +               rmatch.of_node = NULL;
> +               if (of_regulator_match(&pdev->dev, regulators_np, &rmatch,
> +                                      1) != 1) {
> +                       dev_warn(&pdev->dev, "No matching regulator for '%s'\n",
> +                                rmatch.name);
> +                       continue;
> +               }
> +               rdata[i].initdata = rmatch.init_data;
> +               rdata[i].of_node = rmatch.of_node;
> +               rdata[i].id = regulators[i].id;
> +       }
> +
> +       pdata->regulators = rdata;
> +       of_node_put(regulators_np);
> +
> +       return 0;
> +}
> +#else
> +static int max77802_pmic_dt_parse_pdata(struct platform_device *pdev,
> +                                       struct max77686_platform_data *pdata)
> +{
> +       return 0;
> +}
> +#endif /* CONFIG_OF */
> +
> +static int max77802_pmic_probe(struct platform_device *pdev)
> +{
> +       struct max77686_dev *iodev = dev_get_drvdata(pdev->dev.parent);
> +       struct max77686_platform_data *pdata = dev_get_platdata(iodev->dev);
> +       struct max77802_regulator_prv *max77802;
> +       int i, ret = 0, val;
> +       struct regulator_config config = { };
> +
> +       /* This is allocated by the MFD driver */
> +       if (!pdata) {
> +               dev_err(&pdev->dev, "no platform data found for regulator\n");
> +               return -ENODEV;
> +       }
> +
> +       max77802 = devm_kzalloc(&pdev->dev,
> +                               sizeof(struct max77802_regulator_prv),
> +                               GFP_KERNEL);
> +       if (!max77802)
> +               return -ENOMEM;
> +
> +       if (iodev->dev->of_node) {
> +               ret = max77802_pmic_dt_parse_pdata(pdev, pdata);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       config.dev = iodev->dev;
> +       config.regmap = iodev->regmap;
> +       config.driver_data = max77802;
> +       platform_set_drvdata(pdev, max77802);
> +
> +       for (i = 0; i < MAX77802_REG_MAX; i++) {
> +               struct regulator_dev *rdev;
> +               int id = pdata->regulators[i].id;
> +               int shift = max77802_get_opmode_shift(id);
> +
> +               config.init_data = pdata->regulators[i].initdata;
> +               config.of_node = pdata->regulators[i].of_node;
> +
> +               ret = regmap_read(iodev->regmap, regulators[i].enable_reg, &val);
> +               max77802->opmode[id] = val >> shift & MAX77802_OPMODE_MASK;

I have been using this patch series for adding UHS support for dw_mmc
driver. During reboot testing I came across an issue where card
detection fails due to vqmmc regulator not getting enabled. On
debugging further, I found that PMIC driver is reading the operating
mode during probe and reusing it in the enable function. With the UHS
patches vqmmc regulator gets disabled during POWER_OFF and if we do
warm reboot, an incorrect operating mode(OFF) is read. This leads to
the vqmmc regulator staying disabled. I have referred to 77686 driver
and observed that they are handling this a little differently. With
the following change in the driver above issue is resolved:

-               ret = regmap_read(iodev->regmap,
regulators[i].enable_reg, &val);
-               max77802->opmode[id] = val >> shift & MAX77802_OPMODE_MASK;
+               max77802->opmode[i] = regulators[i].enable_mask >> shift;

Please have a look and let me know, if there is any better way of handling this.

> +
> +               rdev = devm_regulator_register(&pdev->dev,
> +                                              &regulators[i], &config);
> +               if (IS_ERR(rdev)) {
> +                       dev_err(&pdev->dev,
> +                               "regulator init failed for %d\n", i);
> +                       return PTR_ERR(rdev);
> +               }
> +       }
> +
> +       return 0;
> +}
> +
> +static const struct platform_device_id max77802_pmic_id[] = {
> +       {"max77802-pmic", 0},
> +       { },
> +};
> +MODULE_DEVICE_TABLE(platform, max77802_pmic_id);
> +
> +static struct platform_driver max77802_pmic_driver = {
> +       .driver = {
> +               .name = "max77802-pmic",
> +               .owner = THIS_MODULE,
> +       },
> +       .probe = max77802_pmic_probe,
> +       .id_table = max77802_pmic_id,
> +};
> +
> +module_platform_driver(max77802_pmic_driver);
> +
> +MODULE_DESCRIPTION("MAXIM 77802 Regulator Driver");
> +MODULE_AUTHOR("Simon Glass <sjg@chromium.org>");
> +MODULE_LICENSE("GPL");
> --
> 2.0.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

[Javier Martinez Canillas]

Hello Yuvaraj,

On 08/22/2014 08:01 AM, Yuvaraj Cd wrote:
>> +
>> +static int max77802_pmic_probe(struct platform_device *pdev)
>> +{
>> +       struct max77686_dev *iodev = dev_get_drvdata(pdev->dev.parent);
>> +       struct max77686_platform_data *pdata = dev_get_platdata(iodev->dev);
>> +       struct max77802_regulator_prv *max77802;
>> +       int i, ret = 0, val;
>> +       struct regulator_config config = { };
>> +
>> +       /* This is allocated by the MFD driver */
>> +       if (!pdata) {
>> +               dev_err(&pdev->dev, "no platform data found for regulator\n");
>> +               return -ENODEV;
>> +       }
>> +
>> +       max77802 = devm_kzalloc(&pdev->dev,
>> +                               sizeof(struct max77802_regulator_prv),
>> +                               GFP_KERNEL);
>> +       if (!max77802)
>> +               return -ENOMEM;
>> +
>> +       if (iodev->dev->of_node) {
>> +               ret = max77802_pmic_dt_parse_pdata(pdev, pdata);
>> +               if (ret)
>> +                       return ret;
>> +       }
>> +
>> +       config.dev = iodev->dev;
>> +       config.regmap = iodev->regmap;
>> +       config.driver_data = max77802;
>> +       platform_set_drvdata(pdev, max77802);
>> +
>> +       for (i = 0; i < MAX77802_REG_MAX; i++) {
>> +               struct regulator_dev *rdev;
>> +               int id = pdata->regulators[i].id;
>> +               int shift = max77802_get_opmode_shift(id);
>> +
>> +               config.init_data = pdata->regulators[i].initdata;
>> +               config.of_node = pdata->regulators[i].of_node;
>> +
>> +               ret = regmap_read(iodev->regmap, regulators[i].enable_reg, &val);
>> +               max77802->opmode[id] = val >> shift & MAX77802_OPMODE_MASK;
> 
> I have been using this patch series for adding UHS support for dw_mmc
> driver. During reboot testing I came across an issue where card
> detection fails due to vqmmc regulator not getting enabled. On
> debugging further, I found that PMIC driver is reading the operating
> mode during probe and reusing it in the enable function. With the UHS
> patches vqmmc regulator gets disabled during POWER_OFF and if we do
> warm reboot, an incorrect operating mode(OFF) is read. This leads to
> the vqmmc regulator staying disabled. I have referred to 77686 driver
> and observed that they are handling this a little differently. With
> the following change in the driver above issue is resolved:
> 
> -               ret = regmap_read(iodev->regmap,
> regulators[i].enable_reg, &val);
> -               max77802->opmode[id] = val >> shift & MAX77802_OPMODE_MASK;
> +               max77802->opmode[i] = regulators[i].enable_mask >> shift;
> 

I don't think this change is correct it its current form since
.enable_mask is initialized to MAX77802_OPMODE_MASK << shift so this is
actually setting the opmode to MAX77802_OPMODE_MASK.

Now, MAX77802_OPMODE_MASK has the same value than MAX77802_OPMODE_NORMAL
so what you are really doing here is initializing the opmode to
MAX77802_OPMODE_NORMAL.

> Please have a look and let me know, if there is any better way of handling this.
>

The first versions of this driver did in fact set the opmode to
MAX77802_OPMODE_NORMAL on probe but Mark asked me to read it from the
hardware instead [0]. I was indeed worried at the time that something like
this could happen on a warm reset [1].

Mark, any opinions on how this should be solved will be highly appreciated.

Best regards,
Javier

[0]: https://lkml.org/lkml/2014/6/16/576
[1]: https://lkml.org/lkml/2014/6/17/174

Re: [PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

[Mark Brown]

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

On Fri, Aug 22, 2014 at 02:15:46PM +0200, Javier Martinez Canillas wrote:

> Mark, any opinions on how this should be solved will be highly appreciated.

If someone could tell me what "this" is that'd help...

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

[Javier Martinez Canillas]

Hello Mark,

On 08/22/2014 04:45 PM, Mark Brown wrote:
> On Fri, Aug 22, 2014 at 02:15:46PM +0200, Javier Martinez Canillas wrote:
> 
>> Mark, any opinions on how this should be solved will be highly appreciated.
> 
> If someone could tell me what "this" is that'd help...
> 

Sorry for not being clear on that regard. By "this" I meant the problem
reported by Yuvaraj.

The regulators operating mode is read from the hardware registers on probe
as you suggested but that means that if the regulator is disabled and the
machine rebooted (warm reset) the opmode reported by the hardware is
MAX77802_OPMODE_OFF.

The problem is that one of these regulators is used as the vqmmc-supply
(VCCQ/VDD_IO) so the mmc host controller driver disables it on
MMC_POWER_OFF. Now AFAIK (Yuvaraj can correct me what I got wrong) this
shouldn't be an issue since on card detection, the vqmmc supply should be
enabled again but on Exynos the built-in card detect line is on the same
power rail as vqmmc. That means that disabling the regulator prevents card
insertions to be detected.

This does not happen on the downstream Chrome OS kernel because the
max77802 regulator driver has some ad-hoc DT bindings were you can define
the operating mode to be set on probe. For this particular regulator is
MAX77802_OPMODE_STANDBY.

Yuvaraj solution was to set the operating mode to MAX77802_OPMODE_NORMAL
on probe() which was what I did on a previous version of the driver but
you explained to me that it was not safe to do that.

That's why I'm asking for suggestions.

Thanks a lot and best regards,
Javier

Re: [PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

[Mark Brown]

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

On Fri, Aug 22, 2014 at 07:53:19PM +0200, Javier Martinez Canillas wrote:
> On 08/22/2014 04:45 PM, Mark Brown wrote:
> > On Fri, Aug 22, 2014 at 02:15:46PM +0200, Javier Martinez Canillas wrote:

> >> Mark, any opinions on how this should be solved will be highly appreciated.

> > If someone could tell me what "this" is that'd help...

> Sorry for not being clear on that regard. By "this" I meant the problem
> reported by Yuvaraj.

That mail was rather lengthy and seemed to be discussing several issues.

> The problem is that one of these regulators is used as the vqmmc-supply
> (VCCQ/VDD_IO) so the mmc host controller driver disables it on
> MMC_POWER_OFF. Now AFAIK (Yuvaraj can correct me what I got wrong) this
> shouldn't be an issue since on card detection, the vqmmc supply should be
> enabled again but on Exynos the built-in card detect line is on the same
> power rail as vqmmc. That means that disabling the regulator prevents card
> insertions to be detected.

If the MMC host controller needs a supply enabling in order to do card
detection and it's supposed to be doing card detection I'd expect it to
be enabling that supply.  Why is it not doing that?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

[Javier Martinez Canillas]

Hello Mark,

On 08/22/2014 08:30 PM, Mark Brown wrote:
> 
>> The problem is that one of these regulators is used as the vqmmc-supply
>> (VCCQ/VDD_IO) so the mmc host controller driver disables it on
>> MMC_POWER_OFF. Now AFAIK (Yuvaraj can correct me what I got wrong) this
>> shouldn't be an issue since on card detection, the vqmmc supply should be
>> enabled again but on Exynos the built-in card detect line is on the same
>> power rail as vqmmc. That means that disabling the regulator prevents card
>> insertions to be detected.
> 
> If the MMC host controller needs a supply enabling in order to do card
> detection and it's supposed to be doing card detection I'd expect it to
> be enabling that supply.  Why is it not doing that?
> 

Good question. I'm not that familiar with the dw_mmc host controller nor
its driver implementation so I'll let Yuvaraj or Doug to answer that.

Best regards,
Javier

Re: [PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

[Doug Anderson]

Hi,

On Fri, Aug 22, 2014 at 3:02 PM, Javier Martinez Canillas
<javier.martinez@collabora.co.uk> wrote:
> Hello Mark,
>
> On 08/22/2014 08:30 PM, Mark Brown wrote:
>>
>>> The problem is that one of these regulators is used as the vqmmc-supply
>>> (VCCQ/VDD_IO) so the mmc host controller driver disables it on
>>> MMC_POWER_OFF. Now AFAIK (Yuvaraj can correct me what I got wrong) this
>>> shouldn't be an issue since on card detection, the vqmmc supply should be
>>> enabled again but on Exynos the built-in card detect line is on the same
>>> power rail as vqmmc. That means that disabling the regulator prevents card
>>> insertions to be detected.
>>
>> If the MMC host controller needs a supply enabling in order to do card
>> detection and it's supposed to be doing card detection I'd expect it to
>> be enabling that supply.  Why is it not doing that?
>>
>
> Good question. I'm not that familiar with the dw_mmc host controller nor
> its driver implementation so I'll let Yuvaraj or Doug to answer that.

I haven't seen the issue that Yuvaraj is reporting (but I also haven't
picked up all of the relevant patches and tried to reproduce), so I'm
going to have to leave it to Yuvaraj to answer.

As far as I know the dw_mmc driver ought to be enabling vqmmc when it
needs it.  Perhaps there's a bug in your patch series that adds vqmmc
support to dw_mmc?

-Doug

Re: [PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

[Yuvaraj Cd]

On Sat, Aug 23, 2014 at 3:45 AM, Doug Anderson <dianders@chromium.org> wrote:
> Hi,
>
> On Fri, Aug 22, 2014 at 3:02 PM, Javier Martinez Canillas
> <javier.martinez@collabora.co.uk> wrote:
>> Hello Mark,
>>
>> On 08/22/2014 08:30 PM, Mark Brown wrote:
>>>
>>>> The problem is that one of these regulators is used as the vqmmc-supply
>>>> (VCCQ/VDD_IO) so the mmc host controller driver disables it on
>>>> MMC_POWER_OFF. Now AFAIK (Yuvaraj can correct me what I got wrong) this
>>>> shouldn't be an issue since on card detection, the vqmmc supply should be
>>>> enabled again but on Exynos the built-in card detect line is on the same
>>>> power rail as vqmmc. That means that disabling the regulator prevents card
>>>> insertions to be detected.
>>>
>>> If the MMC host controller needs a supply enabling in order to do card
>>> detection and it's supposed to be doing card detection I'd expect it to
>>> be enabling that supply.  Why is it not doing that?
>>>
>>
>> Good question. I'm not that familiar with the dw_mmc host controller nor
>> its driver implementation so I'll let Yuvaraj or Doug to answer that.
Well,here it goes!
1. Power ON the board LDO4CTRL1[7:6] 11b
2. dw_mmc driver enable the vqmmc.
3. checks for UHS support, complete the voltage switching t0 1.8V
4. Does warm reset by reboot command.
5. mmc core calls mmc_set_ios() with MMC_POWER_OFF.
6. dw_mmc driver cut-off the regulator with LDO4CTRL1[7:6] is 00b
7.dw_mmc driver enable the vqmmc.
 But after step 7 also, LD4CTRL[7:6] is 00b.
>
> I haven't seen the issue that Yuvaraj is reporting (but I also haven't
> picked up all of the relevant patches and tried to reproduce), so I'm
> going to have to leave it to Yuvaraj to answer.
    static int max77802_enable(struct regulator_dev *rdev)
    {
      struct max77802_regulator_prv *max77802 = rdev_get_drvdata(rdev);
     int id = rdev_get_id(rdev);
     int shift = max77802_get_opmode_shift(id);
     return regmap_update_bits(rdev->regmap,
rdev->desc->enable_reg,rdev->desc->enable_mask,max77802->opmode[id] <<
shift);
  }
I think in the above code snippet, the "val" is what we got it during
the probe.We always write that value for enabling this regulator(which
is LDO4CTRL1[7:6] 00b after warm reset) which is not correct according
the MAX77802 manual.
>
> As far as I know the dw_mmc driver ought to be enabling vqmmc when it
> needs it.  Perhaps there's a bug in your patch series that adds vqmmc
> support to dw_mmc?
>
> -Doug

Re: [PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

[Javier Martinez Canillas]

Hello Yuvaraj,

On 08/25/2014 10:22 AM, Yuvaraj Cd wrote:
>>> Good question. I'm not that familiar with the dw_mmc host controller nor
>>> its driver implementation so I'll let Yuvaraj or Doug to answer that.
> Well,here it goes!
> 1. Power ON the board LDO4CTRL1[7:6] 11b
> 2. dw_mmc driver enable the vqmmc.
> 3. checks for UHS support, complete the voltage switching t0 1.8V
> 4. Does warm reset by reboot command.
> 5. mmc core calls mmc_set_ios() with MMC_POWER_OFF.
> 6. dw_mmc driver cut-off the regulator with LDO4CTRL1[7:6] is 00b
> 7.dw_mmc driver enable the vqmmc.
>  But after step 7 also, LD4CTRL[7:6] is 00b.

Ok, so the dw_mmc driver is enabling vqmmc, that's good.

>>
>> I haven't seen the issue that Yuvaraj is reporting (but I also haven't
>> picked up all of the relevant patches and tried to reproduce), so I'm
>> going to have to leave it to Yuvaraj to answer.
>     static int max77802_enable(struct regulator_dev *rdev)
>     {
>       struct max77802_regulator_prv *max77802 = rdev_get_drvdata(rdev);
>      int id = rdev_get_id(rdev);
>      int shift = max77802_get_opmode_shift(id);
>      return regmap_update_bits(rdev->regmap,
> rdev->desc->enable_reg,rdev->desc->enable_mask,max77802->opmode[id] <<
> shift);
>   }
> I think in the above code snippet, the "val" is what we got it during
> the probe.We always write that value for enabling this regulator(which
> is LDO4CTRL1[7:6] 00b after warm reset) which is not correct according
> the MAX77802 manual.
>>

I see, so probably until we have a way to define the operating mode for
each regulator using DT we should set the opmode to normal when enabling a
regulator independently of the value the hardware register reported on probe.

Can you please test the following change [0] so I can post as a proper
patch? Doug, Mark do you think that forcing the regulator to opmode normal
when enabling is the right solution here?

Best regards,
Javier

[0]
diff --git a/drivers/regulator/max77802.c b/drivers/regulator/max77802.c
index ad1caa9..917b5ab 100644
--- a/drivers/regulator/max77802.c
+++ b/drivers/regulator/max77802.c
@@ -180,7 +180,7 @@ static int max77802_enable(struct regulator_dev *rdev)

        return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
                                  rdev->desc->enable_mask,
-                                 max77802->opmode[id] << shift);
+                                 MAX77802_OPMODE_NORMAL << shift);
 }

Re: [PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

[Yuvaraj Cd]

On Mon, Aug 25, 2014 at 2:37 PM, Javier Martinez Canillas
<javier.martinez@collabora.co.uk> wrote:
> Hello Yuvaraj,
>
> On 08/25/2014 10:22 AM, Yuvaraj Cd wrote:
>>>> Good question. I'm not that familiar with the dw_mmc host controller nor
>>>> its driver implementation so I'll let Yuvaraj or Doug to answer that.
>> Well,here it goes!
>> 1. Power ON the board LDO4CTRL1[7:6] 11b
>> 2. dw_mmc driver enable the vqmmc.
>> 3. checks for UHS support, complete the voltage switching t0 1.8V
>> 4. Does warm reset by reboot command.
>> 5. mmc core calls mmc_set_ios() with MMC_POWER_OFF.
>> 6. dw_mmc driver cut-off the regulator with LDO4CTRL1[7:6] is 00b
>> 7.dw_mmc driver enable the vqmmc.
>>  But after step 7 also, LD4CTRL[7:6] is 00b.
>
> Ok, so the dw_mmc driver is enabling vqmmc, that's good.
>
>>>
>>> I haven't seen the issue that Yuvaraj is reporting (but I also haven't
>>> picked up all of the relevant patches and tried to reproduce), so I'm
>>> going to have to leave it to Yuvaraj to answer.
>>     static int max77802_enable(struct regulator_dev *rdev)
>>     {
>>       struct max77802_regulator_prv *max77802 = rdev_get_drvdata(rdev);
>>      int id = rdev_get_id(rdev);
>>      int shift = max77802_get_opmode_shift(id);
>>      return regmap_update_bits(rdev->regmap,
>> rdev->desc->enable_reg,rdev->desc->enable_mask,max77802->opmode[id] <<
>> shift);
>>   }
>> I think in the above code snippet, the "val" is what we got it during
>> the probe.We always write that value for enabling this regulator(which
>> is LDO4CTRL1[7:6] 00b after warm reset) which is not correct according
>> the MAX77802 manual.
>>>
>
> I see, so probably until we have a way to define the operating mode for
> each regulator using DT we should set the opmode to normal when enabling a
> regulator independently of the value the hardware register reported on probe.
>
> Can you please test the following change [0] so I can post as a proper
> patch? Doug, Mark do you think that forcing the regulator to opmode normal
> when enabling is the right solution here?
>
> Best regards,
> Javier
>
> [0]
> diff --git a/drivers/regulator/max77802.c b/drivers/regulator/max77802.c
> index ad1caa9..917b5ab 100644
> --- a/drivers/regulator/max77802.c
> +++ b/drivers/regulator/max77802.c
> @@ -180,7 +180,7 @@ static int max77802_enable(struct regulator_dev *rdev)
>
>         return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
>                                   rdev->desc->enable_mask,
> -                                 max77802->opmode[id] << shift);
> +                                 MAX77802_OPMODE_NORMAL << shift);
>  }
It works.

Re: [PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

[Doug Anderson]

Javier,

On Mon, Aug 25, 2014 at 2:07 AM, Javier Martinez Canillas
<javier.martinez@collabora.co.uk> wrote:
> Hello Yuvaraj,
>
> On 08/25/2014 10:22 AM, Yuvaraj Cd wrote:
>>>> Good question. I'm not that familiar with the dw_mmc host controller nor
>>>> its driver implementation so I'll let Yuvaraj or Doug to answer that.
>> Well,here it goes!
>> 1. Power ON the board LDO4CTRL1[7:6] 11b
>> 2. dw_mmc driver enable the vqmmc.
>> 3. checks for UHS support, complete the voltage switching t0 1.8V
>> 4. Does warm reset by reboot command.
>> 5. mmc core calls mmc_set_ios() with MMC_POWER_OFF.
>> 6. dw_mmc driver cut-off the regulator with LDO4CTRL1[7:6] is 00b
>> 7.dw_mmc driver enable the vqmmc.
>>  But after step 7 also, LD4CTRL[7:6] is 00b.
>
> Ok, so the dw_mmc driver is enabling vqmmc, that's good.
>
>>>
>>> I haven't seen the issue that Yuvaraj is reporting (but I also haven't
>>> picked up all of the relevant patches and tried to reproduce), so I'm
>>> going to have to leave it to Yuvaraj to answer.
>>     static int max77802_enable(struct regulator_dev *rdev)
>>     {
>>       struct max77802_regulator_prv *max77802 = rdev_get_drvdata(rdev);
>>      int id = rdev_get_id(rdev);
>>      int shift = max77802_get_opmode_shift(id);
>>      return regmap_update_bits(rdev->regmap,
>> rdev->desc->enable_reg,rdev->desc->enable_mask,max77802->opmode[id] <<
>> shift);
>>   }
>> I think in the above code snippet, the "val" is what we got it during
>> the probe.We always write that value for enabling this regulator(which
>> is LDO4CTRL1[7:6] 00b after warm reset) which is not correct according
>> the MAX77802 manual.
>>>
>
> I see, so probably until we have a way to define the operating mode for
> each regulator using DT we should set the opmode to normal when enabling a
> regulator independently of the value the hardware register reported on probe.
>
> Can you please test the following change [0] so I can post as a proper
> patch? Doug, Mark do you think that forcing the regulator to opmode normal
> when enabling is the right solution here?

IMHO that makes sense.

Re: [PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

[Javier Martinez Canillas]

Hello Doug,

On 08/25/2014 05:40 PM, Doug Anderson wrote:
>>
>> I see, so probably until we have a way to define the operating mode for
>> each regulator using DT we should set the opmode to normal when enabling a
>> regulator independently of the value the hardware register reported on probe.
>>
>> Can you please test the following change [0] so I can post as a proper
>> patch? Doug, Mark do you think that forcing the regulator to opmode normal
>> when enabling is the right solution here?
> 
> IMHO that makes sense.
> 

Thanks, I'll post it as a proper patch tomorrow then.

Best regards,
Javier

Re: [PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

[Mark Brown]

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

On Mon, Aug 25, 2014 at 08:40:40AM -0700, Doug Anderson wrote:
> On Mon, Aug 25, 2014 at 2:07 AM, Javier Martinez Canillas

> > I see, so probably until we have a way to define the operating mode for
> > each regulator using DT we should set the opmode to normal when enabling a
> > regulator independently of the value the hardware register reported on probe.

> > Can you please test the following change [0] so I can post as a proper
> > patch? Doug, Mark do you think that forcing the regulator to opmode normal
> > when enabling is the right solution here?

> IMHO that makes sense.

No, this doesn't make any obvious sense to me at all.  Picking normal as
a default if the hardware reads back off due to overlapping
impelementation or something *might* make sense but not overwriting the
hardware state without explicit permission from the machine integration
is a key goal for the regulator API.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

[Javier Martinez Canillas]

Hello Mark,

On 08/26/2014 09:17 AM, Mark Brown wrote:
> On Mon, Aug 25, 2014 at 08:40:40AM -0700, Doug Anderson wrote:
>> > Can you please test the following change [0] so I can post as a proper
>> > patch? Doug, Mark do you think that forcing the regulator to opmode normal
>> > when enabling is the right solution here?
> 
>> IMHO that makes sense.
> 
> No, this doesn't make any obvious sense to me at all.  Picking normal as
> a default if the hardware reads back off due to overlapping
> impelementation or something *might* make sense but not overwriting the
> hardware state without explicit permission from the machine integration
> is a key goal for the regulator API.
> 

Just to be sure I understood you correctly, what might makes sense to you
then is to set the opmode to normal as default on probe only if off is
read back from the hardware register but leaving the enable function as it
is now using the opmode set on probe?

That seems like a safer solution indeed since enable won't overwrite other
values different from off read from the hardware register, I'll prepare a
patch.

Thanks a lot for your help and best regards,
Javier

Re: [PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

[Mark Brown]

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

On Tue, Aug 26, 2014 at 11:08:07AM +0200, Javier Martinez Canillas wrote:
> On 08/26/2014 09:17 AM, Mark Brown wrote:

> > No, this doesn't make any obvious sense to me at all.  Picking normal as
> > a default if the hardware reads back off due to overlapping
> > impelementation or something *might* make sense but not overwriting the
> > hardware state without explicit permission from the machine integration
> > is a key goal for the regulator API.

> Just to be sure I understood you correctly, what might makes sense to you
> then is to set the opmode to normal as default on probe only if off is
> read back from the hardware register but leaving the enable function as it
> is now using the opmode set on probe?

Yes.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]