Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1307142 > unrolled thread

[PATCH V2 6/6] regulator: max77620: add regulator driver for max77620/max20024

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH V2 6/6] regulator: max77620: add regulator driver for max77620/max20024 Laxman Dewangan <ldewangan@nvidia.com> - 2016-01-12 10:30 +0100
    Re: [PATCH V2 6/6] regulator: max77620: add regulator driver for  max77620/max20024 Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-01-13 02:30 +0100
      Re: [PATCH V2 6/6] regulator: max77620: add regulator driver for  max77620/max20024 Mark Brown <broonie@kernel.org> - 2016-01-13 13:00 +0100
        Re: [PATCH V2 6/6] regulator: max77620: add regulator driver for  max77620/max20024 Laxman Dewangan <ldewangan@nvidia.com> - 2016-01-13 13:20 +0100
          Re: [PATCH V2 6/6] regulator: max77620: add regulator driver for  max77620/max20024 Mark Brown <broonie@kernel.org> - 2016-01-13 13:40 +0100
            Re: [PATCH V2 6/6] regulator: max77620: add regulator driver for  max77620/max20024 Laxman Dewangan <ldewangan@nvidia.com> - 2016-01-13 13:50 +0100
              Re: [PATCH V2 6/6] regulator: max77620: add regulator driver for  max77620/max20024 Mark Brown <broonie@kernel.org> - 2016-01-13 14:10 +0100
                Re: [PATCH V2 6/6] regulator: max77620: add regulator driver for  max77620/max20024 Laxman Dewangan <ldewangan@nvidia.com> - 2016-01-13 14:30 +0100
                  Re: [PATCH V2 6/6] regulator: max77620: add regulator driver for  max77620/max20024 Mark Brown <broonie@kernel.org> - 2016-01-13 15:30 +0100
                    Re: [PATCH V2 6/6] regulator: max77620: add regulator driver for  max77620/max20024 Laxman Dewangan <ldewangan@nvidia.com> - 2016-01-13 16:00 +0100
                      Re: [PATCH V2 6/6] regulator: max77620: add regulator driver for  max77620/max20024 Mark Brown <broonie@kernel.org> - 2016-01-13 17:00 +0100
      Re: [PATCH V2 6/6] regulator: max77620: add regulator driver for  max77620/max20024 Laxman Dewangan <ldewangan@nvidia.com> - 2016-01-13 13:00 +0100

#1307142 — [PATCH V2 6/6] regulator: max77620: add regulator driver for max77620/max20024

MAXIM Semiconductor's PMIC, MAX77620 and MAX20024 have the
multiple DCDC and LDOs. This supplies the power to different
components of the system.
Also these rails has configuration for ramp time, flexible
power sequence, slew rate etc.

Add regulator driver to access these rails via regulator APIs.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
Signed-off-by: Mallikarjun Kasoju <mkasoju@nvidia.com>
---
Changes from V1:
- Cleanup code based on comment received on mfd/rtc.
- Avoid duplication on error message.

 drivers/regulator/Kconfig              |   9 +
 drivers/regulator/Makefile             |   1 +
 drivers/regulator/max77620-regulator.c | 991 +++++++++++++++++++++++++++++++++
 3 files changed, 1001 insertions(+)
 create mode 100644 drivers/regulator/max77620-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 8155e80..b92214b 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -343,6 +343,15 @@ config REGULATOR_MAX1586
 	  regulator via I2C bus. The provided regulator is suitable
 	  for PXA27x chips to control VCC_CORE and VCC_USIM voltages.
 
+config REGULATOR_MAX77620
+	tristate "Maxim 77620/MAX20024 voltage regulator"
+	depends on MFD_MAX77620
+	help
+	  This driver controls Maxim MAX77620 voltage output regulator
+	  via I2C bus. The provided regulator is suitable for Tegra
+	  chip to control Step-Down DC-DC and LDOs. Say Y here to
+	  enable the regulator driver.
+
 config REGULATOR_MAX8649
 	tristate "Maxim 8649 voltage regulator"
 	depends on I2C
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 980b194..2564c00 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_REGULATOR_LP8755) += lp8755.o
 obj-$(CONFIG_REGULATOR_LTC3589) += ltc3589.o
 obj-$(CONFIG_REGULATOR_MAX14577) += max14577.o
 obj-$(CONFIG_REGULATOR_MAX1586) += max1586.o
+obj-$(CONFIG_REGULATOR_MAX77620) += max77620-regulator.o
 obj-$(CONFIG_REGULATOR_MAX8649)	+= max8649.o
 obj-$(CONFIG_REGULATOR_MAX8660) += max8660.o
 obj-$(CONFIG_REGULATOR_MAX8907) += max8907-regulator.o
diff --git a/drivers/regulator/max77620-regulator.c b/drivers/regulator/max77620-regulator.c
new file mode 100644
index 0000000..7c89573
--- /dev/null
+++ b/drivers/regulator/max77620-regulator.c
@@ -0,0 +1,991 @@
+/*
+ * Maxim MAX77620 Regulator driver
+ *
+ * Copyright (c) 2016, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * Author: Mallikarjun Kasoju <mkasoju@nvidia.com>
+ *	Laxman Dewangan <ldewangan@nvidia.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/max77620.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/of.h>
+#include <linux/regulator/of_regulator.h>
+
+#define max77620_rails(_name)	"max77620-"#_name
+
+/* Power Mode */
+#define MAX77620_POWER_MODE_NORMAL		3
+#define MAX77620_POWER_MODE_LPM			2
+#define MAX77620_POWER_MODE_GLPM		1
+#define MAX77620_POWER_MODE_DISABLE		0
+
+/* SD Slew Rate */
+#define MAX77620_SD_SR_13_75			0
+#define MAX77620_SD_SR_27_5			1
+#define MAX77620_SD_SR_55			2
+#define MAX77620_SD_SR_100			3
+
+#define MAX77620_FPS_SRC_NUM			3
+
+struct max77620_regulator_info {
+	u8 type;
+	u32 min_uV;
+	u32 max_uV;
+	u32 step_uV;
+	u8 fps_addr;
+	u8 volt_addr;
+	u8 cfg_addr;
+	u8 volt_mask;
+	u8 power_mode_mask;
+	u8 power_mode_shift;
+	u8 remote_sense_addr;
+	u8 remote_sense_mask;
+	struct regulator_desc desc;
+};
+
+struct max77620_regulator_pdata {
+	bool glpm_enable;
+	bool en2_ctrl_sd0;
+	bool sd_fsrade_disable;
+	bool disable_remote_sense_on_suspend;
+	struct regulator_init_data *reg_idata;
+	int active_fps_src;
+	int active_fps_pd_slot;
+	int active_fps_pu_slot;
+	int suspend_fps_src;
+	int suspend_fps_pd_slot;
+	int suspend_fps_pu_slot;
+	int current_mode;
+};
+
+struct max77620_regulator {
+	struct device *dev;
+	struct max77620_chip *max77620_chip;
+	struct max77620_regulator_info *rinfo[MAX77620_NUM_REGS];
+	struct max77620_regulator_pdata reg_pdata[MAX77620_NUM_REGS];
+	struct regulator_dev *rdev[MAX77620_NUM_REGS];
+	int enable_power_mode[MAX77620_NUM_REGS];
+	int current_power_mode[MAX77620_NUM_REGS];
+	int active_fps_src[MAX77620_NUM_REGS];
+};
+
+#define fps_src_name(fps_src)	\
+	(fps_src == FPS_SRC_0 ? "FPS_SRC_0" :	\
+	fps_src == FPS_SRC_1 ? "FPS_SRC_1" :	\
+	fps_src == FPS_SRC_2 ? "FPS_SRC_2" : "FPS_SRC_NONE")
+
+static int max77620_regulator_get_fps_src(struct max77620_regulator *reg,
+		 int id)
+{
+	struct max77620_regulator_info *rinfo = reg->rinfo[id];
+	struct device *parent = reg->max77620_chip->dev;
+	u8 val;
+	int ret;
+
+	ret = max77620_reg_read(parent, MAX77620_PWR_SLAVE,
+			rinfo->fps_addr, &val);
+	if (ret < 0) {
+		dev_err(reg->dev, "Reg 0x%02x read failed %d\n",
+				rinfo->fps_addr, ret);
+		return ret;
+	}
+
+	return (val & MAX77620_FPS_SRC_MASK) >> MAX77620_FPS_SRC_SHIFT;
+}
+
+static int max77620_regulator_set_fps_src(struct max77620_regulator *reg,
+		int fps_src, int id)
+{
+	struct max77620_regulator_info *rinfo = reg->rinfo[id];
+	struct device *parent = reg->max77620_chip->dev;
+	u8 val;
+	int ret;
+
+	switch (fps_src) {
+	case FPS_SRC_0:
+	case FPS_SRC_1:
+	case FPS_SRC_2:
+	case FPS_SRC_NONE:
+		break;
+
+	case FPS_SRC_DEF:
+		ret = max77620_reg_read(parent, MAX77620_PWR_SLAVE,
+			rinfo->fps_addr, &val);
+		if (ret < 0) {
+			dev_err(reg->dev, "Reg 0x%02x read failed %d\n",
+				rinfo->fps_addr, ret);
+			return ret;
+		}
+		ret = (val & MAX77620_FPS_SRC_MASK) >> MAX77620_FPS_SRC_SHIFT;
+		reg->active_fps_src[id] = ret;
+		return 0;
+
+	default:
+		dev_err(reg->dev, "Invalid FPS %d for regulator %d\n",
+			fps_src, id);
+		return -EINVAL;
+	}
+
+	ret = max77620_reg_update(parent, MAX77620_PWR_SLAVE,
+			rinfo->fps_addr, MAX77620_FPS_SRC_MASK,
+			fps_src << MAX77620_FPS_SRC_SHIFT);
+	if (ret < 0) {
+		dev_err(reg->dev, "Reg 0x%02x update failed %d\n",
+			rinfo->fps_addr, ret);
+		return ret;
+	}
+	reg->active_fps_src[id] = fps_src;
+
+	return 0;
+}
+
+static int max77620_regulator_set_fps_slots(struct max77620_regulator *reg,
+			int id, bool is_suspend)
+{
+	struct max77620_regulator_pdata *rpdata = &reg->reg_pdata[id];
+	struct max77620_regulator_info *rinfo = reg->rinfo[id];
+	struct device *parent = reg->max77620_chip->dev;
+	unsigned int val = 0;
+	unsigned int mask = 0;
+	int pu = rpdata->active_fps_pu_slot;
+	int pd = rpdata->active_fps_pd_slot;
+	int ret = 0;
+
+	if (is_suspend) {
+		pu = rpdata->suspend_fps_pu_slot;
+		pd = rpdata->suspend_fps_pd_slot;
+	}
+
+	/* FPS power up period setting */
+	if (pu >= 0) {
+		val |= (pu << MAX77620_FPS_PU_PERIOD_SHIFT);
+		mask |= MAX77620_FPS_PU_PERIOD_MASK;
+	}
+
+	/* FPS power down period setting */
+	if (pd >= 0) {
+		val |= (pd << MAX77620_FPS_PD_PERIOD_SHIFT);
+		mask |= MAX77620_FPS_PD_PERIOD_MASK;
+	}
+
+	if (mask) {
+		ret = max77620_reg_update(parent, MAX77620_PWR_SLAVE,
+				rinfo->fps_addr, mask, val);
+		if (ret < 0) {
+			dev_err(reg->dev, "Reg 0x%02x update failed: %d\n",
+				rinfo->fps_addr, ret);
+			return ret;
+		}
+	}
+
+	return ret;
+}
+
+static int max77620_regulator_set_power_mode(struct max77620_regulator *reg,
+	int power_mode, int id)
+{
+	struct max77620_regulator_info *rinfo = reg->rinfo[id];
+	int ret;
+	struct device *parent = reg->max77620_chip->dev;
+	u8 mask = rinfo->power_mode_mask;
+	u8 shift = rinfo->power_mode_shift;
+	u8 addr;
+
+	switch (rinfo->type) {
+	case MAX77620_REGULATOR_TYPE_SD:
+		addr = rinfo->cfg_addr;
+		break;
+	default:
+		addr = rinfo->volt_addr;
+		break;
+	}
+
+	ret = max77620_reg_update(parent, MAX77620_PWR_SLAVE,
+			addr, mask, power_mode << shift);
+	if (ret < 0) {
+		dev_err(reg->dev, "Regulator %d mode set failed: %d\n",
+			id, ret);
+		return ret;
+	}
+	reg->current_power_mode[id] = power_mode;
+
+	return ret;
+}
+
+static int max77620_regulator_get_power_mode(struct max77620_regulator *reg,
+			int id)
+{
+	struct max77620_regulator_info *rinfo = reg->rinfo[id];
+	struct device *parent = reg->max77620_chip->dev;
+	u8 val;
+	u8 mask = rinfo->power_mode_mask;
+	u8 shift = rinfo->power_mode_shift;
+	u8 addr;
+	int ret;
+
+	switch (rinfo->type) {
+	case MAX77620_REGULATOR_TYPE_SD:
+		addr = rinfo->cfg_addr;
+		break;
+	default:
+		addr = rinfo->volt_addr;
+		break;
+	}
+
+	ret = max77620_reg_read(parent, MAX77620_PWR_SLAVE, addr, &val);
+	if (ret < 0) {
+		dev_err(reg->dev, "Regulator %d: Reg 0x%02x read failed: %d\n",
+			id, addr, ret);
+		return ret;
+	}
+
+	return (val & mask) >> shift;
+}
+
+static int max77620_regulator_enable(struct regulator_dev *rdev)
+{
+	struct max77620_regulator *reg = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	struct max77620_regulator_pdata *rpdata = &reg->reg_pdata[id];
+
+	if (reg->active_fps_src[id] != FPS_SRC_NONE)
+		return 0;
+
+	if ((id == MAX77620_REGULATOR_ID_SD0) && rpdata->en2_ctrl_sd0)
+		return 0;
+
+	return max77620_regulator_set_power_mode(reg,
+			reg->enable_power_mode[id], id);
+}
+
+static int max77620_regulator_disable(struct regulator_dev *rdev)
+{
+	struct max77620_regulator *reg = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	struct max77620_regulator_pdata *rpdata = &reg->reg_pdata[id];
+
+	if (reg->active_fps_src[id] != FPS_SRC_NONE)
+		return 0;
+
+	if ((id == MAX77620_REGULATOR_ID_SD0) && rpdata->en2_ctrl_sd0)
+		return 0;
+
+	return max77620_regulator_set_power_mode(reg,
+			MAX77620_POWER_MODE_DISABLE, id);
+}
+
+static int max77620_regulator_is_enabled(struct regulator_dev *rdev)
+{
+	struct max77620_regulator *reg = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	struct max77620_regulator_pdata *rpdata = &reg->reg_pdata[id];
+	int ret = 1;
+
+	if (reg->active_fps_src[id] != FPS_SRC_NONE)
+		return 1;
+
+	if ((id == MAX77620_REGULATOR_ID_SD0) && rpdata->en2_ctrl_sd0)
+		return 1;
+
+	ret = max77620_regulator_get_power_mode(reg, id);
+	if (ret < 0)
+		return ret;
+
+	if (ret != MAX77620_POWER_MODE_DISABLE)
+		return 1;
+
+	return 0;
+}
+
+static int max77620_regulator_set_mode(struct regulator_dev *rdev,
+				unsigned int mode)
+{
+	struct max77620_regulator *reg = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	struct max77620_regulator_info *rinfo = reg->rinfo[id];
+	struct max77620_regulator_pdata *rpdata = &reg->reg_pdata[id];
+	struct device *parent = reg->max77620_chip->dev;
+	int power_mode;
+	int ret;
+	bool fpwm = false;
+
+	switch (mode) {
+	case REGULATOR_MODE_FAST:
+		fpwm = true;
+		power_mode = MAX77620_POWER_MODE_NORMAL;
+		break;
+
+	case REGULATOR_MODE_NORMAL:
+		power_mode = MAX77620_POWER_MODE_NORMAL;
+		break;
+
+	case REGULATOR_MODE_IDLE:
+		if (rpdata->glpm_enable)
+			power_mode = MAX77620_POWER_MODE_GLPM;
+		else
+			power_mode = MAX77620_POWER_MODE_LPM;
+		break;
+
+	default:
+		dev_err(reg->dev, "Regulator %d mode %d is invalid\n",
+			id, mode);
+		return -EINVAL;
+	}
+
+	if (rinfo->type != MAX77620_REGULATOR_TYPE_SD)
+		goto skip_fpwm;
+
+	if (fpwm)
+		ret = max77620_reg_update(parent, MAX77620_PWR_SLAVE,
+				rinfo->cfg_addr, MAX77620_SD_FPWM_MASK,
+				MAX77620_SD_FPWM_MASK);
+	else
+		ret = max77620_reg_update(parent, MAX77620_PWR_SLAVE,
+				rinfo->cfg_addr, MAX77620_SD_FPWM_MASK, 0);
+	if (ret < 0) {
+		dev_err(reg->dev, "Reg 0x%02x update failed: %d\n",
+				rinfo->cfg_addr, ret);
+		return ret;
+	}
+	rpdata->current_mode = mode;
+
+skip_fpwm:
+	ret = max77620_regulator_set_power_mode(reg, power_mode, id);
+	if (ret < 0)
+		return ret;
+
+	reg->enable_power_mode[id] = power_mode;
+
+	return 0;
+}
+
+static unsigned int max77620_regulator_get_mode(struct regulator_dev *rdev)
+{
+	struct max77620_regulator *reg = rdev_get_drvdata(rdev);
+	struct device *parent = reg->max77620_chip->dev;
+	int id = rdev_get_id(rdev);
+	struct max77620_regulator_info *rinfo = reg->rinfo[id];
+	int fpwm = 0;
+	int ret;
+	int pm_mode, reg_mode;
+	u8 val;
+
+	ret = max77620_regulator_get_power_mode(reg, id);
+	if (ret < 0)
+		return 0;
+
+	pm_mode = ret;
+
+	if (rinfo->type == MAX77620_REGULATOR_TYPE_SD) {
+		ret = max77620_reg_read(parent, MAX77620_PWR_SLAVE,
+				rinfo->cfg_addr, &val);
+		if (ret < 0) {
+			dev_err(reg->dev, "Reg 0x%02x read failed: %d\n",
+				rinfo->cfg_addr, ret);
+			return ret;
+		}
+		fpwm = !!(val & MAX77620_SD_FPWM_MASK);
+	}
+
+	switch (pm_mode) {
+	case MAX77620_POWER_MODE_NORMAL:
+	case MAX77620_POWER_MODE_DISABLE:
+		if (fpwm)
+			reg_mode = REGULATOR_MODE_FAST;
+		else
+			reg_mode = REGULATOR_MODE_NORMAL;
+		break;
+	case MAX77620_POWER_MODE_LPM:
+	case MAX77620_POWER_MODE_GLPM:
+		reg_mode = REGULATOR_MODE_IDLE;
+		break;
+	default:
+		return 0;
+	}
+
+	return reg_mode;
+}
+
+static int max77620_regulator_set_ramp_delay(struct regulator_dev *rdev,
+			int ramp_delay)
+{
+	struct max77620_regulator *reg = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	struct max77620_regulator_info *rinfo = reg->rinfo[id];
+	struct device *parent = reg->max77620_chip->dev;
+	int ret, val;
+	u8 mask;
+
+	if (rinfo->type == MAX77620_REGULATOR_TYPE_SD) {
+		if (ramp_delay <= 13750)
+			val = 0;
+		else if (ramp_delay <= 27500)
+			val = 1;
+		else if (ramp_delay <= 55000)
+			val = 2;
+		else
+			val = 3;
+		val <<= MAX77620_SD_SR_SHIFT;
+		mask = MAX77620_SD_SR_MASK;
+	} else {
+		if (ramp_delay <= 5000)
+			val = 1;
+		else
+			val = 0;
+		mask = MAX77620_LDO_SLEW_RATE_MASK;
+	}
+
+	ret = max77620_reg_update(parent, MAX77620_PWR_SLAVE,
+				rinfo->cfg_addr, mask, val);
+	if (ret < 0)
+		dev_err(reg->dev, "Reg 0x%02x update failed: %d\n",
+				rinfo->cfg_addr, ret);
+
+	return ret;
+}
+
+static struct regulator_ops max77620_regulator_ops = {
+	.is_enabled = max77620_regulator_is_enabled,
+	.enable = max77620_regulator_enable,
+	.disable = max77620_regulator_disable,
+	.list_voltage = regulator_list_voltage_linear,
+	.map_voltage = regulator_map_voltage_linear,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.set_mode = max77620_regulator_set_mode,
+	.get_mode = max77620_regulator_get_mode,
+	.set_ramp_delay = max77620_regulator_set_ramp_delay,
+	.set_voltage_time_sel = regulator_set_voltage_time_sel,
+};
+
+#define MAX77620_SD_CNF2_ROVS_EN_NONE	0
+#define RAIL_SD(_id, _name, _sname, _volt_mask, _min_uV, _max_uV,	\
+		_step_uV, _rs_add, _rs_mask)				\
+	[MAX77620_REGULATOR_ID_##_id] = {			\
+		.type = MAX77620_REGULATOR_TYPE_SD,			\
+		.volt_mask = MAX77620_##_volt_mask##_VOLT_MASK,	\
+		.volt_addr = MAX77620_REG_##_id,		\
+		.cfg_addr = MAX77620_REG_##_id##_CFG,		\
+		.fps_addr = MAX77620_REG_FPS_##_id,		\
+		.remote_sense_addr = _rs_add,			\
+		.remote_sense_mask = MAX77620_SD_CNF2_ROVS_EN_##_rs_mask, \
+		.min_uV = _min_uV,				\
+		.max_uV = _max_uV,				\
+		.step_uV = _step_uV,				\
+		.power_mode_mask = MAX77620_SD_POWER_MODE_MASK,		\
+		.power_mode_shift = MAX77620_SD_POWER_MODE_SHIFT,	\
+		.desc = {					\
+			.name = max77620_rails(_name),		\
+			.supply_name = _sname,			\
+			.id = MAX77620_REGULATOR_ID_##_id,	\
+			.ops = &max77620_regulator_ops,		\
+			.n_voltages = ((_max_uV - _min_uV) / _step_uV) + 1, \
+			.min_uV = _min_uV,	\
+			.uV_step = _step_uV,	\
+			.enable_time = 500,	\
+			.vsel_mask = MAX77620_##_volt_mask##_VOLT_MASK,	\
+			.vsel_reg = MAX77620_REG_##_id,	\
+			.type = REGULATOR_VOLTAGE,	\
+		},						\
+	}
+
+#define RAIL_LDO(_id, _name, _sname, _type, _min_uV, _max_uV, _step_uV) \
+	[MAX77620_REGULATOR_ID_##_id] = {			\
+		.type = MAX77620_REGULATOR_TYPE_LDO_##_type,		\
+		.volt_mask = MAX77620_LDO_VOLT_MASK,			\
+		.volt_addr = MAX77620_REG_##_id##_CFG,		\
+		.cfg_addr = MAX77620_REG_##_id##_CFG2,		\
+		.fps_addr = MAX77620_REG_FPS_##_id,		\
+		.remote_sense_addr = 0xFF,			\
+		.min_uV = _min_uV,				\
+		.max_uV = _max_uV,				\
+		.step_uV = _step_uV,				\
+		.power_mode_mask = MAX77620_LDO_POWER_MODE_MASK,	\
+		.power_mode_shift = MAX77620_LDO_POWER_MODE_SHIFT,	\
+		.desc = {					\
+			.name = max77620_rails(_name),		\
+			.supply_name = _sname,			\
+			.id = MAX77620_REGULATOR_ID_##_id,	\
+			.ops = &max77620_regulator_ops,		\
+			.n_voltages = ((_max_uV - _min_uV) / _step_uV) + 1, \
+			.min_uV = _min_uV,	\
+			.uV_step = _step_uV,	\
+			.enable_time = 500,	\
+			.vsel_mask = MAX77620_LDO_VOLT_MASK,	\
+			.vsel_reg = MAX77620_REG_##_id##_CFG, \
+			.type = REGULATOR_VOLTAGE,		\
+		},						\
+	}
+
+static struct max77620_regulator_info max77620_regs_info[MAX77620_NUM_REGS] = {
+	RAIL_SD(SD0, sd0, "in-sd0", SD0, 600000, 1400000, 12500, 0x22, SD0),
+	RAIL_SD(SD1, sd1, "in-sd1", SD1, 600000, 1550000, 12500, 0x22, SD1),
+	RAIL_SD(SD2, sd2, "in-sd2", SDX, 600000, 3787500, 12500, 0xFF, NONE),
+	RAIL_SD(SD3, sd3, "in-sd3", SDX, 600000, 3787500, 12500, 0xFF, NONE),
+	RAIL_SD(SD4, sd4, "in-sd4", SDX, 600000, 3787500, 12500, 0xFF, NONE),
+
+	RAIL_LDO(LDO0, ldo0, "in-ldo0-1", N, 800000, 2375000, 25000),
+	RAIL_LDO(LDO1, ldo1, "in-ldo0-1", N, 800000, 2375000, 25000),
+	RAIL_LDO(LDO2, ldo2, "in-ldo2",   P, 800000, 3950000, 50000),
+	RAIL_LDO(LDO3, ldo3, "in-ldo3-5", P, 800000, 3950000, 50000),
+	RAIL_LDO(LDO4, ldo4, "in-ldo4-6", P, 800000, 1587500, 12500),
+	RAIL_LDO(LDO5, ldo5, "in-ldo3-5", P, 800000, 3950000, 50000),
+	RAIL_LDO(LDO6, ldo6, "in-ldo4-6", P, 800000, 3950000, 50000),
+	RAIL_LDO(LDO7, ldo7, "in-ldo7-8", N, 800000, 3950000, 50000),
+	RAIL_LDO(LDO8, ldo8, "in-ldo7-8", N, 800000, 3950000, 50000),
+};
+
+static struct max77620_regulator_info max20024_regs_info[MAX77620_NUM_REGS] = {
+	RAIL_SD(SD0, sd0, "in-sd0", SD0, 800000, 1587500, 12500, 0x22, SD0),
+	RAIL_SD(SD1, sd1, "in-sd1", SD1, 600000, 3387500, 12500, 0x22, SD1),
+	RAIL_SD(SD2, sd2, "in-sd2", SDX, 600000, 3787500, 12500, 0xFF, NONE),
+	RAIL_SD(SD3, sd3, "in-sd3", SDX, 600000, 3787500, 12500, 0xFF, NONE),
+	RAIL_SD(SD4, sd4, "in-sd4", SDX, 600000, 3787500, 12500, 0xFF, NONE),
+
+	RAIL_LDO(LDO0, ldo0, "in-ldo0-1", N, 800000, 2375000, 25000),
+	RAIL_LDO(LDO1, ldo1, "in-ldo0-1", N, 800000, 2375000, 25000),
+	RAIL_LDO(LDO2, ldo2, "in-ldo2",   P, 800000, 3950000, 50000),
+	RAIL_LDO(LDO3, ldo3, "in-ldo3-5", P, 800000, 3950000, 50000),
+	RAIL_LDO(LDO4, ldo4, "in-ldo4-6", P, 800000, 1587500, 12500),
+	RAIL_LDO(LDO5, ldo5, "in-ldo3-5", P, 800000, 3950000, 50000),
+	RAIL_LDO(LDO6, ldo6, "in-ldo4-6", P, 800000, 3950000, 50000),
+	RAIL_LDO(LDO7, ldo7, "in-ldo7-8", N, 800000, 3950000, 50000),
+	RAIL_LDO(LDO8, ldo8, "in-ldo7-8", N, 800000, 3950000, 50000),
+};
+
+static struct of_regulator_match max77620_regulator_matches[] = {
+	{ .name = "sd0", },
+	{ .name = "sd1", },
+	{ .name = "sd2", },
+	{ .name = "sd3", },
+	{ .name = "sd4", },
+	{ .name = "ldo0", },
+	{ .name = "ldo1", },
+	{ .name = "ldo2", },
+	{ .name = "ldo3", },
+	{ .name = "ldo4", },
+	{ .name = "ldo5", },
+	{ .name = "ldo6", },
+	{ .name = "ldo7", },
+	{ .name = "ldo8", },
+};
+
+static int max77620_regulator_preinit(struct max77620_regulator *reg, int id)
+{
+	struct max77620_regulator_pdata *rpdata = &reg->reg_pdata[id];
+	struct max77620_regulator_info *rinfo = reg->rinfo[id];
+	struct device *parent = reg->max77620_chip->dev;
+	struct regulator_init_data *ridata = reg->reg_pdata[id].reg_idata;
+	struct regulator_desc *rdesc = &max77620_regs_info[id].desc;
+	struct device *dev = reg->dev;
+	int init_uv;
+	u8 val, mask, rval;
+	int slew_rate;
+	int ret;
+
+	if (!ridata)
+		return 0;
+
+	/* Update power mode */
+	ret = max77620_regulator_get_power_mode(reg, id);
+	if (ret < 0)
+		return ret;
+
+	reg->current_power_mode[id] = ret;
+	reg->enable_power_mode[id] = MAX77620_POWER_MODE_NORMAL;
+
+	if (rpdata->active_fps_src == FPS_SRC_DEF) {
+		ret = max77620_regulator_get_fps_src(reg, id);
+		if (ret < 0)
+			return ret;
+		rpdata->active_fps_src = ret;
+	}
+
+	/*
+	 * If rails are externally control of FPS control then enable it
+	 * always.
+	 */
+	if ((rpdata->active_fps_src != FPS_SRC_NONE) &&
+		(reg->current_power_mode[id] != reg->enable_power_mode[id])) {
+		ret = max77620_regulator_set_power_mode(reg,
+				reg->enable_power_mode[id], id);
+		if (ret < 0)
+			return ret;
+	}
+
+	/* Enable rail before changing FPS to NONE to avoid glitch */
+	if (ridata && ridata->constraints.boot_on &&
+		(rpdata->active_fps_src == FPS_SRC_NONE)) {
+		init_uv = ridata->constraints.min_uV;
+		if (init_uv && (init_uv == ridata->constraints.max_uV)) {
+			val = (init_uv - rdesc->min_uV) / rdesc->uV_step;
+			ret = max77620_reg_update(parent, MAX77620_PWR_SLAVE,
+					rdesc->vsel_reg, rdesc->vsel_mask, val);
+			if (ret < 0) {
+				dev_err(dev, "Reg 0x%02x update failed: %d\n",
+					rdesc->vsel_reg, ret);
+				return ret;
+			}
+		}
+
+		ret = max77620_regulator_set_power_mode(reg,
+				reg->enable_power_mode[id], id);
+		if (ret < 0)
+			return ret;
+	}
+
+	ret = max77620_regulator_set_fps_src(reg, rpdata->active_fps_src, id);
+	if (ret < 0)
+		return ret;
+
+	ret = max77620_regulator_set_fps_slots(reg, id, false);
+	if (ret < 0)
+		return ret;
+
+	switch (rinfo->type) {
+	case MAX77620_REGULATOR_TYPE_SD:
+		ret = max77620_reg_read(parent, MAX77620_PWR_SLAVE,
+				rinfo->cfg_addr, &rval);
+		if (ret < 0) {
+			dev_err(reg->dev, "Register 0x%02x read failed: %d\n",
+					rinfo->cfg_addr, ret);
+			return ret;
+		}
+
+		slew_rate = (rval >> MAX77620_SD_SR_SHIFT) & 0x3;
+		switch (slew_rate) {
+		case 0:
+			slew_rate = 13750;
+			break;
+		case 1:
+			slew_rate = 27500;
+			break;
+		case 2:
+			slew_rate = 55000;
+			break;
+		case 3:
+			slew_rate = 100000;
+			break;
+		}
+		rinfo->desc.ramp_delay = slew_rate;
+
+		mask = MAX77620_SD_FSRADE_MASK;
+		val = 0;
+		if (rpdata->sd_fsrade_disable)
+			val |= MAX77620_SD_FSRADE_MASK;
+
+		ret = max77620_reg_update(parent, MAX77620_PWR_SLAVE,
+				rinfo->cfg_addr, mask, val);
+		if (ret < 0) {
+			dev_err(reg->dev, "Reg 0x%02x update failed: %d\n",
+				rinfo->cfg_addr, ret);
+			return ret;
+		}
+		break;
+	default:
+		ret = max77620_reg_read(parent, MAX77620_PWR_SLAVE,
+				rinfo->cfg_addr, &rval);
+		if (ret < 0) {
+			dev_err(reg->dev, "Register 0x%02x read failed: %d\n",
+					rinfo->cfg_addr, ret);
+			return ret;
+		}
+		slew_rate = rval & 0x1;
+		switch (slew_rate) {
+		case 0:
+			slew_rate = 100000;
+			break;
+		case 1:
+			slew_rate = 5000;
+			break;
+		}
+		rinfo->desc.ramp_delay = slew_rate;
+		break;
+	}
+
+	if ((id == MAX77620_REGULATOR_ID_SD0) && rpdata->en2_ctrl_sd0) {
+		ret = max77620_regulator_set_power_mode(reg,
+				MAX77620_POWER_MODE_NORMAL, id);
+		if (!ret)
+			ret = max77620_regulator_set_fps_src(reg, FPS_SRC_0,
+					id);
+	}
+
+	return ret;
+}
+
+static int max77620_get_regulator_dt_data(struct platform_device *pdev,
+		struct max77620_regulator *max77620_regs)
+{
+	struct device_node *np;
+	u32 prop;
+	int id;
+	int ret;
+
+	np = of_get_child_by_name(pdev->dev.parent->of_node, "regulators");
+	if (!np) {
+		dev_err(&pdev->dev, "Device is not having regulators node\n");
+		return -ENODEV;
+	}
+
+	ret = of_regulator_match(&pdev->dev, np, max77620_regulator_matches,
+			ARRAY_SIZE(max77620_regulator_matches));
+	if (ret < 0) {
+		dev_err(&pdev->dev, "regulator node parsing failed: %d\n", ret);
+		return ret;
+	}
+
+	for (id = 0; id < ARRAY_SIZE(max77620_regulator_matches); ++id) {
+		struct device_node *reg_node;
+		struct max77620_regulator_pdata *reg_pdata =
+					&max77620_regs->reg_pdata[id];
+
+		reg_node = max77620_regulator_matches[id].of_node;
+		reg_pdata->reg_idata = max77620_regulator_matches[id].init_data;
+
+		if (!reg_pdata->reg_idata)
+			continue;
+
+		reg_pdata->glpm_enable = of_property_read_bool(reg_node,
+					"maxim,enable-group-low-power");
+
+		reg_pdata->en2_ctrl_sd0 = of_property_read_bool(reg_node,
+					"maxim,enable-sd0-en2-control");
+
+		reg_pdata->sd_fsrade_disable = of_property_read_bool(reg_node,
+					"maxim,disable-active-discharge");
+
+		reg_pdata->disable_remote_sense_on_suspend =
+				of_property_read_bool(reg_node,
+				"maxim,disable-remote-sense-on-suspend");
+
+		ret = of_property_read_u32(reg_node,
+				"maxim,active-fps-source", &prop);
+		if (!ret)
+			reg_pdata->active_fps_src = prop;
+		else
+			reg_pdata->active_fps_src = FPS_SRC_DEF;
+
+		ret = of_property_read_u32(reg_node,
+				"maxim,active-fps-power-up-slot", &prop);
+		if (!ret)
+			reg_pdata->active_fps_pu_slot = prop;
+		else
+			reg_pdata->active_fps_pu_slot = -1;
+
+		ret = of_property_read_u32(reg_node,
+				"maxim,active-fps-power-down-slot", &prop);
+		if (!ret)
+			reg_pdata->active_fps_pd_slot = prop;
+		else
+			reg_pdata->active_fps_pd_slot = -1;
+
+		reg_pdata->suspend_fps_src = -1;
+		reg_pdata->suspend_fps_pu_slot = -1;
+		reg_pdata->suspend_fps_pd_slot = -1;
+		ret = of_property_read_u32(reg_node,
+				"maxim,suspend-fps-source", &prop);
+		if (!ret)
+			reg_pdata->suspend_fps_src = prop;
+
+		ret = of_property_read_u32(reg_node,
+				"maxim,suspend-fps-power-up-slot", &prop);
+		if (!ret)
+			reg_pdata->suspend_fps_pu_slot = prop;
+
+		ret = of_property_read_u32(reg_node,
+				"maxim,suspend-fps-power-down-slot", &prop);
+		if (!ret)
+			reg_pdata->suspend_fps_pd_slot = prop;
+	}
+
+	return 0;
+}
+
+static int max77620_regulator_probe(struct platform_device *pdev)
+{
+	struct max77620_chip *max77620_chip = dev_get_drvdata(pdev->dev.parent);
+	struct regulator_desc *rdesc;
+	struct max77620_regulator *pmic;
+	struct regulator_config config = { };
+	struct max77620_regulator_info *regulator_info;
+	int ret = 0;
+	int id;
+
+	pmic = devm_kzalloc(&pdev->dev, sizeof(*pmic), GFP_KERNEL);
+	if (!pmic)
+		return -ENOMEM;
+
+	max77620_get_regulator_dt_data(pdev, pmic);
+
+	platform_set_drvdata(pdev, pmic);
+	pmic->max77620_chip = max77620_chip;
+	pmic->dev = &pdev->dev;
+
+	if (max77620_chip->id == MAX77620)
+		regulator_info = max77620_regs_info;
+	else
+		regulator_info = max20024_regs_info;
+
+	for (id = 0; id < MAX77620_NUM_REGS; ++id) {
+
+		if ((max77620_chip->id == MAX77620) &&
+			(id == MAX77620_REGULATOR_ID_SD4))
+			continue;
+
+		rdesc = &regulator_info[id].desc;
+		pmic->rinfo[id] = &max77620_regs_info[id];
+		pmic->enable_power_mode[id] = MAX77620_POWER_MODE_NORMAL;
+
+		config.regmap = max77620_chip->rmap[MAX77620_PWR_SLAVE];
+		config.dev = &pdev->dev;
+		config.init_data = pmic->reg_pdata[id].reg_idata;
+		config.driver_data = pmic;
+		config.of_node = max77620_regulator_matches[id].of_node;
+
+		ret = max77620_regulator_preinit(pmic, id);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "Preinit regualtor %s failed: %d\n",
+				rdesc->name, ret);
+			return ret;
+		}
+
+		pmic->rdev[id] = devm_regulator_register(&pdev->dev,
+						rdesc, &config);
+		if (IS_ERR(pmic->rdev[id])) {
+			ret = PTR_ERR(pmic->rdev[id]);
+			dev_err(&pdev->dev, "Reg registeration %s failed: %d\n",
+				rdesc->name, ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int max77620_regulator_suspend(struct device *dev)
+{
+	struct max77620_regulator *pmic = dev_get_drvdata(dev);
+	struct max77620_regulator_pdata *reg_pdata;
+	struct max77620_regulator_info *rinfo;
+	struct device *parent = pmic->dev->parent;
+	int id;
+	int ret;
+
+	for (id = 0; id < MAX77620_NUM_REGS; ++id) {
+		reg_pdata = &pmic->reg_pdata[id];
+		rinfo = pmic->rinfo[id];
+		if (!reg_pdata || !rinfo)
+			continue;
+
+		if (reg_pdata->disable_remote_sense_on_suspend &&
+			(rinfo->remote_sense_addr != 0xFF)) {
+			ret = max77620_reg_update(parent, MAX77620_PWR_SLAVE,
+				rinfo->remote_sense_addr,
+				rinfo->remote_sense_mask, 0);
+			if (ret < 0)
+				dev_err(dev, "Reg 0x%02x update failed: %d\n",
+					rinfo->remote_sense_addr, ret);
+		}
+
+		max77620_regulator_set_fps_slots(pmic, id, true);
+		if (reg_pdata->suspend_fps_src >= 0)
+			max77620_regulator_set_fps_src(pmic,
+				reg_pdata->suspend_fps_src, id);
+	}
+
+	return 0;
+}
+
+static int max77620_regulator_resume(struct device *dev)
+{
+	struct max77620_regulator *pmic = dev_get_drvdata(dev);
+	struct max77620_regulator_pdata *reg_pdata;
+	struct max77620_regulator_info *rinfo;
+	struct device *parent = pmic->dev->parent;
+	int id;
+	int ret;
+
+	for (id = 0; id < MAX77620_NUM_REGS; ++id) {
+		reg_pdata = &pmic->reg_pdata[id];
+		rinfo = pmic->rinfo[id];
+		if (!reg_pdata || !rinfo)
+			continue;
+
+		if (reg_pdata->disable_remote_sense_on_suspend &&
+			(rinfo->remote_sense_addr != 0xFF)) {
+			ret = max77620_reg_update(parent, MAX77620_PWR_SLAVE,
+				rinfo->remote_sense_addr,
+				rinfo->remote_sense_mask,
+				rinfo->remote_sense_mask);
+			if (ret < 0)
+				dev_err(dev, "Reg 0x%02x update failed: %d\n",
+					rinfo->remote_sense_addr, ret);
+		}
+
+		max77620_regulator_set_fps_slots(pmic, id, false);
+		if (reg_pdata->active_fps_src >= 0)
+			max77620_regulator_set_fps_src(pmic,
+				reg_pdata->active_fps_src, id);
+	}
+
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops max77620_regulator_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(max77620_regulator_suspend,
+			max77620_regulator_resume)
+};
+
+static const struct platform_device_id max77620_regulator_devtype[] = {
+	{
+		.name = "max77620-pmic",
+	},
+	{
+		.name = "max20024-pmic",
+	},
+};
+
+static struct platform_driver max77620_regulator_driver = {
+	.probe = max77620_regulator_probe,
+	.id_table = max77620_regulator_devtype,
+	.driver = {
+		.name = "max77620-pmic",
+		.pm = &max77620_regulator_pm_ops,
+	},
+};
+
+static int __init max77620_regulator_init(void)
+{
+	return platform_driver_register(&max77620_regulator_driver);
+}
+subsys_initcall(max77620_regulator_init);
+
+static void __exit max77620_reg_exit(void)
+{
+	platform_driver_unregister(&max77620_regulator_driver);
+}
+module_exit(max77620_reg_exit);
+
+MODULE_DESCRIPTION("MAX77620/MAX20024 regulator driver");
+MODULE_AUTHOR("Mallikarjun Kasoju <mkasoju@nvidia.com>");
+MODULE_AUTHOR("Laxman Dewangan <ldewangan@nvidia.com>");
+MODULE_ALIAS("platform:max77620-pmic");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

[toc] | [next] | [standalone]

#1307957 — Re: [PATCH V2 6/6] regulator: max77620: add regulator driver for max77620/max20024

On 12.01.2016 18:17, Laxman Dewangan wrote:
> MAXIM Semiconductor's PMIC, MAX77620 and MAX20024 have the
> multiple DCDC and LDOs. This supplies the power to different
> components of the system.
> Also these rails has configuration for ramp time, flexible
> power sequence, slew rate etc.
> 
> Add regulator driver to access these rails via regulator APIs.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> Signed-off-by: Mallikarjun Kasoju <mkasoju@nvidia.com>
> ---
> Changes from V1:
> - Cleanup code based on comment received on mfd/rtc.
> - Avoid duplication on error message.
> 
>  drivers/regulator/Kconfig              |   9 +
>  drivers/regulator/Makefile             |   1 +
>  drivers/regulator/max77620-regulator.c | 991 +++++++++++++++++++++++++++++++++
>  3 files changed, 1001 insertions(+)
>  create mode 100644 drivers/regulator/max77620-regulator.c
> 
> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> index 8155e80..b92214b 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -343,6 +343,15 @@ config REGULATOR_MAX1586
>  	  regulator via I2C bus. The provided regulator is suitable
>  	  for PXA27x chips to control VCC_CORE and VCC_USIM voltages.
>  
> +config REGULATOR_MAX77620
> +	tristate "Maxim 77620/MAX20024 voltage regulator"
> +	depends on MFD_MAX77620
> +	help
> +	  This driver controls Maxim MAX77620 voltage output regulator
> +	  via I2C bus. The provided regulator is suitable for Tegra
> +	  chip to control Step-Down DC-DC and LDOs. Say Y here to
> +	  enable the regulator driver.
> +
>  config REGULATOR_MAX8649
>  	tristate "Maxim 8649 voltage regulator"
>  	depends on I2C
> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
> index 980b194..2564c00 100644
> --- a/drivers/regulator/Makefile
> +++ b/drivers/regulator/Makefile
> @@ -46,6 +46,7 @@ obj-$(CONFIG_REGULATOR_LP8755) += lp8755.o
>  obj-$(CONFIG_REGULATOR_LTC3589) += ltc3589.o
>  obj-$(CONFIG_REGULATOR_MAX14577) += max14577.o
>  obj-$(CONFIG_REGULATOR_MAX1586) += max1586.o
> +obj-$(CONFIG_REGULATOR_MAX77620) += max77620-regulator.o
>  obj-$(CONFIG_REGULATOR_MAX8649)	+= max8649.o
>  obj-$(CONFIG_REGULATOR_MAX8660) += max8660.o
>  obj-$(CONFIG_REGULATOR_MAX8907) += max8907-regulator.o
> diff --git a/drivers/regulator/max77620-regulator.c b/drivers/regulator/max77620-regulator.c
> new file mode 100644
> index 0000000..7c89573
> --- /dev/null
> +++ b/drivers/regulator/max77620-regulator.c
> @@ -0,0 +1,991 @@
> +/*
> + * Maxim MAX77620 Regulator driver
> + *
> + * Copyright (c) 2016, NVIDIA CORPORATION.  All rights reserved.
> + *
> + * Author: Mallikarjun Kasoju <mkasoju@nvidia.com>
> + *	Laxman Dewangan <ldewangan@nvidia.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/core.h>

Do you need kernel.h?
mfd/core.h looks unused.

> +#include <linux/mfd/max77620.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/regulator/machine.h>
> +#include <linux/of.h>
> +#include <linux/regulator/of_regulator.h>
> +
> +#define max77620_rails(_name)	"max77620-"#_name
> +
> +/* Power Mode */
> +#define MAX77620_POWER_MODE_NORMAL		3
> +#define MAX77620_POWER_MODE_LPM			2
> +#define MAX77620_POWER_MODE_GLPM		1
> +#define MAX77620_POWER_MODE_DISABLE		0
> +
> +/* SD Slew Rate */
> +#define MAX77620_SD_SR_13_75			0
> +#define MAX77620_SD_SR_27_5			1
> +#define MAX77620_SD_SR_55			2
> +#define MAX77620_SD_SR_100			3
> +
> +#define MAX77620_FPS_SRC_NUM			3
> +
> +struct max77620_regulator_info {
> +	u8 type;
> +	u32 min_uV;
> +	u32 max_uV;
> +	u32 step_uV;
> +	u8 fps_addr;
> +	u8 volt_addr;
> +	u8 cfg_addr;
> +	u8 volt_mask;
> +	u8 power_mode_mask;
> +	u8 power_mode_shift;
> +	u8 remote_sense_addr;
> +	u8 remote_sense_mask;
> +	struct regulator_desc desc;
> +};
> +
> +struct max77620_regulator_pdata {
> +	bool glpm_enable;
> +	bool en2_ctrl_sd0;
> +	bool sd_fsrade_disable;
> +	bool disable_remote_sense_on_suspend;
> +	struct regulator_init_data *reg_idata;
> +	int active_fps_src;
> +	int active_fps_pd_slot;
> +	int active_fps_pu_slot;
> +	int suspend_fps_src;
> +	int suspend_fps_pd_slot;
> +	int suspend_fps_pu_slot;
> +	int current_mode;
> +};
> +
> +struct max77620_regulator {
> +	struct device *dev;
> +	struct max77620_chip *max77620_chip;
> +	struct max77620_regulator_info *rinfo[MAX77620_NUM_REGS];
> +	struct max77620_regulator_pdata reg_pdata[MAX77620_NUM_REGS];
> +	struct regulator_dev *rdev[MAX77620_NUM_REGS];

You don't need the rdev. You are not using it anyway.


> +	int enable_power_mode[MAX77620_NUM_REGS];
> +	int current_power_mode[MAX77620_NUM_REGS];
> +	int active_fps_src[MAX77620_NUM_REGS];
> +};
> +
> +#define fps_src_name(fps_src)	\
> +	(fps_src == FPS_SRC_0 ? "FPS_SRC_0" :	\
> +	fps_src == FPS_SRC_1 ? "FPS_SRC_1" :	\
> +	fps_src == FPS_SRC_2 ? "FPS_SRC_2" : "FPS_SRC_NONE")
> +
> +static int max77620_regulator_get_fps_src(struct max77620_regulator *reg,
> +		 int id)
> +{
> +	struct max77620_regulator_info *rinfo = reg->rinfo[id];
> +	struct device *parent = reg->max77620_chip->dev;
> +	u8 val;
> +	int ret;
> +
> +	ret = max77620_reg_read(parent, MAX77620_PWR_SLAVE,
> +			rinfo->fps_addr, &val);
> +	if (ret < 0) {
> +		dev_err(reg->dev, "Reg 0x%02x read failed %d\n",
> +				rinfo->fps_addr, ret);
> +		return ret;
> +	}
> +
> +	return (val & MAX77620_FPS_SRC_MASK) >> MAX77620_FPS_SRC_SHIFT;
> +}
> +
> +static int max77620_regulator_set_fps_src(struct max77620_regulator *reg,
> +		int fps_src, int id)
> +{
> +	struct max77620_regulator_info *rinfo = reg->rinfo[id];
> +	struct device *parent = reg->max77620_chip->dev;
> +	u8 val;
> +	int ret;
> +
> +	switch (fps_src) {
> +	case FPS_SRC_0:
> +	case FPS_SRC_1:
> +	case FPS_SRC_2:
> +	case FPS_SRC_NONE:
> +		break;
> +
> +	case FPS_SRC_DEF:
> +		ret = max77620_reg_read(parent, MAX77620_PWR_SLAVE,
> +			rinfo->fps_addr, &val);
> +		if (ret < 0) {
> +			dev_err(reg->dev, "Reg 0x%02x read failed %d\n",
> +				rinfo->fps_addr, ret);
> +			return ret;
> +		}
> +		ret = (val & MAX77620_FPS_SRC_MASK) >> MAX77620_FPS_SRC_SHIFT;
> +		reg->active_fps_src[id] = ret;
> +		return 0;
> +
> +	default:
> +		dev_err(reg->dev, "Invalid FPS %d for regulator %d\n",
> +			fps_src, id);
> +		return -EINVAL;
> +	}
> +
> +	ret = max77620_reg_update(parent, MAX77620_PWR_SLAVE,
> +			rinfo->fps_addr, MAX77620_FPS_SRC_MASK,
> +			fps_src << MAX77620_FPS_SRC_SHIFT);
> +	if (ret < 0) {
> +		dev_err(reg->dev, "Reg 0x%02x update failed %d\n",
> +			rinfo->fps_addr, ret);
> +		return ret;
> +	}
> +	reg->active_fps_src[id] = fps_src;
> +
> +	return 0;
> +}
> +
> +static int max77620_regulator_set_fps_slots(struct max77620_regulator *reg,
> +			int id, bool is_suspend)
> +{
> +	struct max77620_regulator_pdata *rpdata = &reg->reg_pdata[id];
> +	struct max77620_regulator_info *rinfo = reg->rinfo[id];
> +	struct device *parent = reg->max77620_chip->dev;
> +	unsigned int val = 0;
> +	unsigned int mask = 0;
> +	int pu = rpdata->active_fps_pu_slot;
> +	int pd = rpdata->active_fps_pd_slot;
> +	int ret = 0;
> +
> +	if (is_suspend) {
> +		pu = rpdata->suspend_fps_pu_slot;
> +		pd = rpdata->suspend_fps_pd_slot;
> +	}
> +
> +	/* FPS power up period setting */
> +	if (pu >= 0) {
> +		val |= (pu << MAX77620_FPS_PU_PERIOD_SHIFT);
> +		mask |= MAX77620_FPS_PU_PERIOD_MASK;
> +	}
> +
> +	/* FPS power down period setting */
> +	if (pd >= 0) {
> +		val |= (pd << MAX77620_FPS_PD_PERIOD_SHIFT);
> +		mask |= MAX77620_FPS_PD_PERIOD_MASK;
> +	}
> +
> +	if (mask) {
> +		ret = max77620_reg_update(parent, MAX77620_PWR_SLAVE,
> +				rinfo->fps_addr, mask, val);
> +		if (ret < 0) {
> +			dev_err(reg->dev, "Reg 0x%02x update failed: %d\n",
> +				rinfo->fps_addr, ret);
> +			return ret;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static int max77620_regulator_set_power_mode(struct max77620_regulator *reg,
> +	int power_mode, int id)
> +{
> +	struct max77620_regulator_info *rinfo = reg->rinfo[id];
> +	int ret;

Could you put the 'ret' at the end of these variables?

> +	struct device *parent = reg->max77620_chip->dev;
> +	u8 mask = rinfo->power_mode_mask;
> +	u8 shift = rinfo->power_mode_shift;
> +	u8 addr;
> +
> +	switch (rinfo->type) {
> +	case MAX77620_REGULATOR_TYPE_SD:
> +		addr = rinfo->cfg_addr;
> +		break;
> +	default:
> +		addr = rinfo->volt_addr;
> +		break;
> +	}
> +
> +	ret = max77620_reg_update(parent, MAX77620_PWR_SLAVE,
> +			addr, mask, power_mode << shift);
> +	if (ret < 0) {
> +		dev_err(reg->dev, "Regulator %d mode set failed: %d\n",
> +			id, ret);
> +		return ret;
> +	}
> +	reg->current_power_mode[id] = power_mode;
> +
> +	return ret;
> +}
> +
> +static int max77620_regulator_get_power_mode(struct max77620_regulator *reg,
> +			int id)
> +{
> +	struct max77620_regulator_info *rinfo = reg->rinfo[id];
> +	struct device *parent = reg->max77620_chip->dev;
> +	u8 val;

Another nit: 'val' after 'u8 addr'.

> +	u8 mask = rinfo->power_mode_mask;
> +	u8 shift = rinfo->power_mode_shift;
> +	u8 addr;
> +	int ret;
> +
> +	switch (rinfo->type) {
> +	case MAX77620_REGULATOR_TYPE_SD:
> +		addr = rinfo->cfg_addr;
> +		break;
> +	default:
> +		addr = rinfo->volt_addr;
> +		break;
> +	}
> +
> +	ret = max77620_reg_read(parent, MAX77620_PWR_SLAVE, addr, &val);
> +	if (ret < 0) {
> +		dev_err(reg->dev, "Regulator %d: Reg 0x%02x read failed: %d\n",
> +			id, addr, ret);
> +		return ret;
> +	}
> +
> +	return (val & mask) >> shift;
> +}
> +
> +static int max77620_regulator_enable(struct regulator_dev *rdev)
> +{
> +	struct max77620_regulator *reg = rdev_get_drvdata(rdev);
> +	int id = rdev_get_id(rdev);
> +	struct max77620_regulator_pdata *rpdata = &reg->reg_pdata[id];
> +
> +	if (reg->active_fps_src[id] != FPS_SRC_NONE)
> +		return 0;
> +
> +	if ((id == MAX77620_REGULATOR_ID_SD0) && rpdata->en2_ctrl_sd0)
> +		return 0;
> +
> +	return max77620_regulator_set_power_mode(reg,
> +			reg->enable_power_mode[id], id);
> +}
> +
> +static int max77620_regulator_disable(struct regulator_dev *rdev)
> +{
> +	struct max77620_regulator *reg = rdev_get_drvdata(rdev);
> +	int id = rdev_get_id(rdev);
> +	struct max77620_regulator_pdata *rpdata = &reg->reg_pdata[id];
> +
> +	if (reg->active_fps_src[id] != FPS_SRC_NONE)
> +		return 0;
> +
> +	if ((id == MAX77620_REGULATOR_ID_SD0) && rpdata->en2_ctrl_sd0)
> +		return 0;
> +
> +	return max77620_regulator_set_power_mode(reg,
> +			MAX77620_POWER_MODE_DISABLE, id);
> +}
> +
> +static int max77620_regulator_is_enabled(struct regulator_dev *rdev)
> +{
> +	struct max77620_regulator *reg = rdev_get_drvdata(rdev);
> +	int id = rdev_get_id(rdev);
> +	struct max77620_regulator_pdata *rpdata = &reg->reg_pdata[id];
> +	int ret = 1;
> +
> +	if (reg->active_fps_src[id] != FPS_SRC_NONE)
> +		return 1;
> +
> +	if ((id == MAX77620_REGULATOR_ID_SD0) && rpdata->en2_ctrl_sd0)
> +		return 1;
> +
> +	ret = max77620_regulator_get_power_mode(reg, id);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (ret != MAX77620_POWER_MODE_DISABLE)
> +		return 1;
> +
> +	return 0;
> +}
> +
> +static int max77620_regulator_set_mode(struct regulator_dev *rdev,
> +				unsigned int mode)
> +{
> +	struct max77620_regulator *reg = rdev_get_drvdata(rdev);
> +	int id = rdev_get_id(rdev);
> +	struct max77620_regulator_info *rinfo = reg->rinfo[id];
> +	struct max77620_regulator_pdata *rpdata = &reg->reg_pdata[id];
> +	struct device *parent = reg->max77620_chip->dev;
> +	int power_mode;
> +	int ret;
> +	bool fpwm = false;
> +
> +	switch (mode) {
> +	case REGULATOR_MODE_FAST:
> +		fpwm = true;
> +		power_mode = MAX77620_POWER_MODE_NORMAL;
> +		break;
> +
> +	case REGULATOR_MODE_NORMAL:
> +		power_mode = MAX77620_POWER_MODE_NORMAL;
> +		break;
> +
> +	case REGULATOR_MODE_IDLE:
> +		if (rpdata->glpm_enable)
> +			power_mode = MAX77620_POWER_MODE_GLPM;
> +		else
> +			power_mode = MAX77620_POWER_MODE_LPM;
> +		break;
> +
> +	default:
> +		dev_err(reg->dev, "Regulator %d mode %d is invalid\n",
> +			id, mode);
> +		return -EINVAL;
> +	}
> +
> +	if (rinfo->type != MAX77620_REGULATOR_TYPE_SD)
> +		goto skip_fpwm;
> +
> +	if (fpwm)
> +		ret = max77620_reg_update(parent, MAX77620_PWR_SLAVE,
> +				rinfo->cfg_addr, MAX77620_SD_FPWM_MASK,
> +				MAX77620_SD_FPWM_MASK);
> +	else
> +		ret = max77620_reg_update(parent, MAX77620_PWR_SLAVE,
> +				rinfo->cfg_addr, MAX77620_SD_FPWM_MASK, 0);

Instead something like:

u8 fwpm_val;
if (fpwm)
        fwpm_val = MAX77620_SD_FPWM_MASK;
else
        fwpm_val = 0;

max77620_reg_update(..., fwpm_val);

You could actually combine it with 'fwpm' variable (replacing it) but I
am not sure if that would make the code more readable. But definitely
one place of calling max77620_reg_update() would be nice.

> +	if (ret < 0) {
> +		dev_err(reg->dev, "Reg 0x%02x update failed: %d\n",
> +				rinfo->cfg_addr, ret);
> +		return ret;
> +	}
> +	rpdata->current_mode = mode;
> +
> +skip_fpwm:
> +	ret = max77620_regulator_set_power_mode(reg, power_mode, id);
> +	if (ret < 0)

You are not reverting the changes made before - setting the fwpm. The
entire operation is not atomic. I am not sure whether that is important
or not... up to you actually.

> +		return ret;
> +
> +	reg->enable_power_mode[id] = power_mode;
> +
> +	return 0;
> +}
> +
> +static unsigned int max77620_regulator_get_mode(struct regulator_dev *rdev)
> +{
> +	struct max77620_regulator *reg = rdev_get_drvdata(rdev);
> +	struct device *parent = reg->max77620_chip->dev;
> +	int id = rdev_get_id(rdev);
> +	struct max77620_regulator_info *rinfo = reg->rinfo[id];
> +	int fpwm = 0;
> +	int ret;
> +	int pm_mode, reg_mode;
> +	u8 val;
> +
> +	ret = max77620_regulator_get_power_mode(reg, id);
> +	if (ret < 0)
> +		return 0;
> +
> +	pm_mode = ret;
> +
> +	if (rinfo->type == MAX77620_REGULATOR_TYPE_SD) {
> +		ret = max77620_reg_read(parent, MAX77620_PWR_SLAVE,
> +				rinfo->cfg_addr, &val);
> +		if (ret < 0) {
> +			dev_err(reg->dev, "Reg 0x%02x read failed: %d\n",
> +				rinfo->cfg_addr, ret);
> +			return ret;
> +		}
> +		fpwm = !!(val & MAX77620_SD_FPWM_MASK);
> +	}
> +
> +	switch (pm_mode) {
> +	case MAX77620_POWER_MODE_NORMAL:
> +	case MAX77620_POWER_MODE_DISABLE:
> +		if (fpwm)
> +			reg_mode = REGULATOR_MODE_FAST;
> +		else
> +			reg_mode = REGULATOR_MODE_NORMAL;
> +		break;
> +	case MAX77620_POWER_MODE_LPM:
> +	case MAX77620_POWER_MODE_GLPM:
> +		reg_mode = REGULATOR_MODE_IDLE;
> +		break;
> +	default:
> +		return 0;
> +	}
> +
> +	return reg_mode;
> +}
> +
> +static int max77620_regulator_set_ramp_delay(struct regulator_dev *rdev,
> +			int ramp_delay)
> +{
> +	struct max77620_regulator *reg = rdev_get_drvdata(rdev);
> +	int id = rdev_get_id(rdev);
> +	struct max77620_regulator_info *rinfo = reg->rinfo[id];
> +	struct device *parent = reg->max77620_chip->dev;
> +	int ret, val;
> +	u8 mask;
> +
> +	if (rinfo->type == MAX77620_REGULATOR_TYPE_SD) {
> +		if (ramp_delay <= 13750)
> +			val = 0;
> +		else if (ramp_delay <= 27500)
> +			val = 1;
> +		else if (ramp_delay <= 55000)
> +			val = 2;
> +		else
> +			val = 3;
> +		val <<= MAX77620_SD_SR_SHIFT;
> +		mask = MAX77620_SD_SR_MASK;
> +	} else {
> +		if (ramp_delay <= 5000)
> +			val = 1;
> +		else
> +			val = 0;
> +		mask = MAX77620_LDO_SLEW_RATE_MASK;
> +	}
> +
> +	ret = max77620_reg_update(parent, MAX77620_PWR_SLAVE,
> +				rinfo->cfg_addr, mask, val);
> +	if (ret < 0)
> +		dev_err(reg->dev, "Reg 0x%02x update failed: %d\n",
> +				rinfo->cfg_addr, ret);
> +
> +	return ret;
> +}
> +
> +static struct regulator_ops max77620_regulator_ops = {
> +	.is_enabled = max77620_regulator_is_enabled,
> +	.enable = max77620_regulator_enable,
> +	.disable = max77620_regulator_disable,
> +	.list_voltage = regulator_list_voltage_linear,
> +	.map_voltage = regulator_map_voltage_linear,
> +	.get_voltage_sel = regulator_get_voltage_sel_regmap,
> +	.set_voltage_sel = regulator_set_voltage_sel_regmap,
> +	.set_mode = max77620_regulator_set_mode,
> +	.get_mode = max77620_regulator_get_mode,
> +	.set_ramp_delay = max77620_regulator_set_ramp_delay,
> +	.set_voltage_time_sel = regulator_set_voltage_time_sel,
> +};
> +
> +#define MAX77620_SD_CNF2_ROVS_EN_NONE	0
> +#define RAIL_SD(_id, _name, _sname, _volt_mask, _min_uV, _max_uV,	\
> +		_step_uV, _rs_add, _rs_mask)				\
> +	[MAX77620_REGULATOR_ID_##_id] = {			\
> +		.type = MAX77620_REGULATOR_TYPE_SD,			\
> +		.volt_mask = MAX77620_##_volt_mask##_VOLT_MASK,	\
> +		.volt_addr = MAX77620_REG_##_id,		\
> +		.cfg_addr = MAX77620_REG_##_id##_CFG,		\
> +		.fps_addr = MAX77620_REG_FPS_##_id,		\
> +		.remote_sense_addr = _rs_add,			\
> +		.remote_sense_mask = MAX77620_SD_CNF2_ROVS_EN_##_rs_mask, \
> +		.min_uV = _min_uV,				\
> +		.max_uV = _max_uV,				\
> +		.step_uV = _step_uV,				\
> +		.power_mode_mask = MAX77620_SD_POWER_MODE_MASK,		\
> +		.power_mode_shift = MAX77620_SD_POWER_MODE_SHIFT,	\
> +		.desc = {					\
> +			.name = max77620_rails(_name),		\
> +			.supply_name = _sname,			\
> +			.id = MAX77620_REGULATOR_ID_##_id,	\
> +			.ops = &max77620_regulator_ops,		\
> +			.n_voltages = ((_max_uV - _min_uV) / _step_uV) + 1, \
> +			.min_uV = _min_uV,	\
> +			.uV_step = _step_uV,	\
> +			.enable_time = 500,	\
> +			.vsel_mask = MAX77620_##_volt_mask##_VOLT_MASK,	\
> +			.vsel_reg = MAX77620_REG_##_id,	\
> +			.type = REGULATOR_VOLTAGE,	\
> +		},						\
> +	}
> +
> +#define RAIL_LDO(_id, _name, _sname, _type, _min_uV, _max_uV, _step_uV) \
> +	[MAX77620_REGULATOR_ID_##_id] = {			\
> +		.type = MAX77620_REGULATOR_TYPE_LDO_##_type,		\
> +		.volt_mask = MAX77620_LDO_VOLT_MASK,			\
> +		.volt_addr = MAX77620_REG_##_id##_CFG,		\
> +		.cfg_addr = MAX77620_REG_##_id##_CFG2,		\
> +		.fps_addr = MAX77620_REG_FPS_##_id,		\
> +		.remote_sense_addr = 0xFF,			\
> +		.min_uV = _min_uV,				\
> +		.max_uV = _max_uV,				\
> +		.step_uV = _step_uV,				\
> +		.power_mode_mask = MAX77620_LDO_POWER_MODE_MASK,	\
> +		.power_mode_shift = MAX77620_LDO_POWER_MODE_SHIFT,	\
> +		.desc = {					\
> +			.name = max77620_rails(_name),		\
> +			.supply_name = _sname,			\
> +			.id = MAX77620_REGULATOR_ID_##_id,	\
> +			.ops = &max77620_regulator_ops,		\
> +			.n_voltages = ((_max_uV - _min_uV) / _step_uV) + 1, \
> +			.min_uV = _min_uV,	\
> +			.uV_step = _step_uV,	\
> +			.enable_time = 500,	\

Here and before please put the line concatenator '\' aligned with
previous, so:
+			.min_uV = _min_uV,			\


> +			.vsel_mask = MAX77620_LDO_VOLT_MASK,	\
> +			.vsel_reg = MAX77620_REG_##_id##_CFG, \
> +			.type = REGULATOR_VOLTAGE,		\
> +		},						\
> +	}
> +
> +static struct max77620_regulator_info max77620_regs_info[MAX77620_NUM_REGS] = {
> +	RAIL_SD(SD0, sd0, "in-sd0", SD0, 600000, 1400000, 12500, 0x22, SD0),
> +	RAIL_SD(SD1, sd1, "in-sd1", SD1, 600000, 1550000, 12500, 0x22, SD1),
> +	RAIL_SD(SD2, sd2, "in-sd2", SDX, 600000, 3787500, 12500, 0xFF, NONE),
> +	RAIL_SD(SD3, sd3, "in-sd3", SDX, 600000, 3787500, 12500, 0xFF, NONE),
> +	RAIL_SD(SD4, sd4, "in-sd4", SDX, 600000, 3787500, 12500, 0xFF, NONE),
> +
> +	RAIL_LDO(LDO0, ldo0, "in-ldo0-1", N, 800000, 2375000, 25000),
> +	RAIL_LDO(LDO1, ldo1, "in-ldo0-1", N, 800000, 2375000, 25000),
> +	RAIL_LDO(LDO2, ldo2, "in-ldo2",   P, 800000, 3950000, 50000),
> +	RAIL_LDO(LDO3, ldo3, "in-ldo3-5", P, 800000, 3950000, 50000),
> +	RAIL_LDO(LDO4, ldo4, "in-ldo4-6", P, 800000, 1587500, 12500),
> +	RAIL_LDO(LDO5, ldo5, "in-ldo3-5", P, 800000, 3950000, 50000),
> +	RAIL_LDO(LDO6, ldo6, "in-ldo4-6", P, 800000, 3950000, 50000),
> +	RAIL_LDO(LDO7, ldo7, "in-ldo7-8", N, 800000, 3950000, 50000),
> +	RAIL_LDO(LDO8, ldo8, "in-ldo7-8", N, 800000, 3950000, 50000),
> +};
> +
> +static struct max77620_regulator_info max20024_regs_info[MAX77620_NUM_REGS] = {
> +	RAIL_SD(SD0, sd0, "in-sd0", SD0, 800000, 1587500, 12500, 0x22, SD0),
> +	RAIL_SD(SD1, sd1, "in-sd1", SD1, 600000, 3387500, 12500, 0x22, SD1),
> +	RAIL_SD(SD2, sd2, "in-sd2", SDX, 600000, 3787500, 12500, 0xFF, NONE),
> +	RAIL_SD(SD3, sd3, "in-sd3", SDX, 600000, 3787500, 12500, 0xFF, NONE),
> +	RAIL_SD(SD4, sd4, "in-sd4", SDX, 600000, 3787500, 12500, 0xFF, NONE),
> +
> +	RAIL_LDO(LDO0, ldo0, "in-ldo0-1", N, 800000, 2375000, 25000),
> +	RAIL_LDO(LDO1, ldo1, "in-ldo0-1", N, 800000, 2375000, 25000),
> +	RAIL_LDO(LDO2, ldo2, "in-ldo2",   P, 800000, 3950000, 50000),
> +	RAIL_LDO(LDO3, ldo3, "in-ldo3-5", P, 800000, 3950000, 50000),
> +	RAIL_LDO(LDO4, ldo4, "in-ldo4-6", P, 800000, 1587500, 12500),
> +	RAIL_LDO(LDO5, ldo5, "in-ldo3-5", P, 800000, 3950000, 50000),
> +	RAIL_LDO(LDO6, ldo6, "in-ldo4-6", P, 800000, 3950000, 50000),
> +	RAIL_LDO(LDO7, ldo7, "in-ldo7-8", N, 800000, 3950000, 50000),
> +	RAIL_LDO(LDO8, ldo8, "in-ldo7-8", N, 800000, 3950000, 50000),
> +};
> +
> +static struct of_regulator_match max77620_regulator_matches[] = {
> +	{ .name = "sd0", },
> +	{ .name = "sd1", },
> +	{ .name = "sd2", },
> +	{ .name = "sd3", },
> +	{ .name = "sd4", },
> +	{ .name = "ldo0", },
> +	{ .name = "ldo1", },
> +	{ .name = "ldo2", },
> +	{ .name = "ldo3", },
> +	{ .name = "ldo4", },
> +	{ .name = "ldo5", },
> +	{ .name = "ldo6", },
> +	{ .name = "ldo7", },
> +	{ .name = "ldo8", },
> +};
> +
> +static int max77620_regulator_preinit(struct max77620_regulator *reg, int id)
> +{
> +	struct max77620_regulator_pdata *rpdata = &reg->reg_pdata[id];
> +	struct max77620_regulator_info *rinfo = reg->rinfo[id];
> +	struct device *parent = reg->max77620_chip->dev;
> +	struct regulator_init_data *ridata = reg->reg_pdata[id].reg_idata;
> +	struct regulator_desc *rdesc = &max77620_regs_info[id].desc;
> +	struct device *dev = reg->dev;
> +	int init_uv;
> +	u8 val, mask, rval;
> +	int slew_rate;
> +	int ret;
> +
> +	if (!ridata)
> +		return 0;
> +
> +	/* Update power mode */
> +	ret = max77620_regulator_get_power_mode(reg, id);
> +	if (ret < 0)
> +		return ret;
> +
> +	reg->current_power_mode[id] = ret;
> +	reg->enable_power_mode[id] = MAX77620_POWER_MODE_NORMAL;
> +
> +	if (rpdata->active_fps_src == FPS_SRC_DEF) {
> +		ret = max77620_regulator_get_fps_src(reg, id);
> +		if (ret < 0)
> +			return ret;
> +		rpdata->active_fps_src = ret;
> +	}
> +
> +	/*
> +	 * If rails are externally control of FPS control then enable it
> +	 * always.
> +	 */
> +	if ((rpdata->active_fps_src != FPS_SRC_NONE) &&
> +		(reg->current_power_mode[id] != reg->enable_power_mode[id])) {
> +		ret = max77620_regulator_set_power_mode(reg,
> +				reg->enable_power_mode[id], id);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	/* Enable rail before changing FPS to NONE to avoid glitch */
> +	if (ridata && ridata->constraints.boot_on &&
> +		(rpdata->active_fps_src == FPS_SRC_NONE)) {
> +		init_uv = ridata->constraints.min_uV;
> +		if (init_uv && (init_uv == ridata->constraints.max_uV)) {
> +			val = (init_uv - rdesc->min_uV) / rdesc->uV_step;
> +			ret = max77620_reg_update(parent, MAX77620_PWR_SLAVE,
> +					rdesc->vsel_reg, rdesc->vsel_mask, val);
> +			if (ret < 0) {
> +				dev_err(dev, "Reg 0x%02x update failed: %d\n",
> +					rdesc->vsel_reg, ret);
> +				return ret;
> +			}
> +		}
> +
> +		ret = max77620_regulator_set_power_mode(reg,
> +				reg->enable_power_mode[id], id);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	ret = max77620_regulator_set_fps_src(reg, rpdata->active_fps_src, id);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = max77620_regulator_set_fps_slots(reg, id, false);
> +	if (ret < 0)
> +		return ret;
> +
> +	switch (rinfo->type) {
> +	case MAX77620_REGULATOR_TYPE_SD:
> +		ret = max77620_reg_read(parent, MAX77620_PWR_SLAVE,
> +				rinfo->cfg_addr, &rval);
> +		if (ret < 0) {
> +			dev_err(reg->dev, "Register 0x%02x read failed: %d\n",
> +					rinfo->cfg_addr, ret);
> +			return ret;
> +		}
> +
> +		slew_rate = (rval >> MAX77620_SD_SR_SHIFT) & 0x3;
> +		switch (slew_rate) {
> +		case 0:
> +			slew_rate = 13750;
> +			break;
> +		case 1:
> +			slew_rate = 27500;
> +			break;
> +		case 2:
> +			slew_rate = 55000;
> +			break;
> +		case 3:
> +			slew_rate = 100000;
> +			break;
> +		}
> +		rinfo->desc.ramp_delay = slew_rate;
> +
> +		mask = MAX77620_SD_FSRADE_MASK;
> +		val = 0;
> +		if (rpdata->sd_fsrade_disable)
> +			val |= MAX77620_SD_FSRADE_MASK;
> +
> +		ret = max77620_reg_update(parent, MAX77620_PWR_SLAVE,
> +				rinfo->cfg_addr, mask, val);
> +		if (ret < 0) {
> +			dev_err(reg->dev, "Reg 0x%02x update failed: %d\n",
> +				rinfo->cfg_addr, ret);
> +			return ret;
> +		}
> +		break;
> +	default:
> +		ret = max77620_reg_read(parent, MAX77620_PWR_SLAVE,
> +				rinfo->cfg_addr, &rval);
> +		if (ret < 0) {
> +			dev_err(reg->dev, "Register 0x%02x read failed: %d\n",
> +					rinfo->cfg_addr, ret);
> +			return ret;
> +		}
> +		slew_rate = rval & 0x1;
> +		switch (slew_rate) {
> +		case 0:
> +			slew_rate = 100000;
> +			break;
> +		case 1:
> +			slew_rate = 5000;
> +			break;
> +		}
> +		rinfo->desc.ramp_delay = slew_rate;
> +		break;
> +	}
> +
> +	if ((id == MAX77620_REGULATOR_ID_SD0) && rpdata->en2_ctrl_sd0) {
> +		ret = max77620_regulator_set_power_mode(reg,
> +				MAX77620_POWER_MODE_NORMAL, id);
> +		if (!ret)
> +			ret = max77620_regulator_set_fps_src(reg, FPS_SRC_0,
> +					id);
> +	}
> +
> +	return ret;
> +}
> +
> +static int max77620_get_regulator_dt_data(struct platform_device *pdev,
> +		struct max77620_regulator *max77620_regs)
> +{
> +	struct device_node *np;
> +	u32 prop;
> +	int id;
> +	int ret;
> +
> +	np = of_get_child_by_name(pdev->dev.parent->of_node, "regulators");
> +	if (!np) {
> +		dev_err(&pdev->dev, "Device is not having regulators node\n");
> +		return -ENODEV;
> +	}
> +
> +	ret = of_regulator_match(&pdev->dev, np, max77620_regulator_matches,
> +			ARRAY_SIZE(max77620_regulator_matches));

Why not using 'regulators_node' and getting rid of this and some code
below? If you need to parse custom regulator properties use 'of_parse_cb'.

> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "regulator node parsing failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	for (id = 0; id < ARRAY_SIZE(max77620_regulator_matches); ++id) {
> +		struct device_node *reg_node;
> +		struct max77620_regulator_pdata *reg_pdata =
> +					&max77620_regs->reg_pdata[id];
> +
> +		reg_node = max77620_regulator_matches[id].of_node;
> +		reg_pdata->reg_idata = max77620_regulator_matches[id].init_data;
> +
> +		if (!reg_pdata->reg_idata)
> +			continue;
> +
> +		reg_pdata->glpm_enable = of_property_read_bool(reg_node,
> +					"maxim,enable-group-low-power");
> +
> +		reg_pdata->en2_ctrl_sd0 = of_property_read_bool(reg_node,
> +					"maxim,enable-sd0-en2-control");
> +
> +		reg_pdata->sd_fsrade_disable = of_property_read_bool(reg_node,
> +					"maxim,disable-active-discharge");
> +
> +		reg_pdata->disable_remote_sense_on_suspend =
> +				of_property_read_bool(reg_node,
> +				"maxim,disable-remote-sense-on-suspend");
> +
> +		ret = of_property_read_u32(reg_node,
> +				"maxim,active-fps-source", &prop);
> +		if (!ret)
> +			reg_pdata->active_fps_src = prop;
> +		else
> +			reg_pdata->active_fps_src = FPS_SRC_DEF;
> +
> +		ret = of_property_read_u32(reg_node,
> +				"maxim,active-fps-power-up-slot", &prop);
> +		if (!ret)
> +			reg_pdata->active_fps_pu_slot = prop;
> +		else
> +			reg_pdata->active_fps_pu_slot = -1;
> +
> +		ret = of_property_read_u32(reg_node,
> +				"maxim,active-fps-power-down-slot", &prop);
> +		if (!ret)
> +			reg_pdata->active_fps_pd_slot = prop;
> +		else
> +			reg_pdata->active_fps_pd_slot = -1;
> +
> +		reg_pdata->suspend_fps_src = -1;
> +		reg_pdata->suspend_fps_pu_slot = -1;
> +		reg_pdata->suspend_fps_pd_slot = -1;
> +		ret = of_property_read_u32(reg_node,
> +				"maxim,suspend-fps-source", &prop);
> +		if (!ret)
> +			reg_pdata->suspend_fps_src = prop;
> +
> +		ret = of_property_read_u32(reg_node,
> +				"maxim,suspend-fps-power-up-slot", &prop);
> +		if (!ret)
> +			reg_pdata->suspend_fps_pu_slot = prop;
> +
> +		ret = of_property_read_u32(reg_node,
> +				"maxim,suspend-fps-power-down-slot", &prop);
> +		if (!ret)
> +			reg_pdata->suspend_fps_pd_slot = prop;
> +	}
> +
> +	return 0;
> +}
> +
> +static int max77620_regulator_probe(struct platform_device *pdev)
> +{
> +	struct max77620_chip *max77620_chip = dev_get_drvdata(pdev->dev.parent);
> +	struct regulator_desc *rdesc;

This is should be local to the for-loop,

> +	struct max77620_regulator *pmic;
> +	struct regulator_config config = { };
> +	struct max77620_regulator_info *regulator_info;
> +	int ret = 0;
> +	int id;
> +
> +	pmic = devm_kzalloc(&pdev->dev, sizeof(*pmic), GFP_KERNEL);
> +	if (!pmic)
> +		return -ENOMEM;
> +
> +	max77620_get_regulator_dt_data(pdev, pmic);
> +
> +	platform_set_drvdata(pdev, pmic);
> +	pmic->max77620_chip = max77620_chip;
> +	pmic->dev = &pdev->dev;
> +
> +	if (max77620_chip->id == MAX77620)
> +		regulator_info = max77620_regs_info;
> +	else
> +		regulator_info = max20024_regs_info;
> +
> +	for (id = 0; id < MAX77620_NUM_REGS; ++id) {
> +
> +		if ((max77620_chip->id == MAX77620) &&
> +			(id == MAX77620_REGULATOR_ID_SD4))
> +			continue;
> +
> +		rdesc = &regulator_info[id].desc;
> +		pmic->rinfo[id] = &max77620_regs_info[id];
> +		pmic->enable_power_mode[id] = MAX77620_POWER_MODE_NORMAL;
> +
> +		config.regmap = max77620_chip->rmap[MAX77620_PWR_SLAVE];
> +		config.dev = &pdev->dev;
> +		config.init_data = pmic->reg_pdata[id].reg_idata;
> +		config.driver_data = pmic;

Don't do it for each regulator. Set the config.regmap/dev/driver_data
once before the loop.

Each regulator_register() makes a copy of passed 'config'.

> +		config.of_node = max77620_regulator_matches[id].of_node;
> +
> +		ret = max77620_regulator_preinit(pmic, id);
> +		if (ret < 0) {
> +			dev_err(&pdev->dev, "Preinit regualtor %s failed: %d\n",
> +				rdesc->name, ret);
> +			return ret;
> +		}
> +
> +		pmic->rdev[id] = devm_regulator_register(&pdev->dev,
> +						rdesc, &config);
> +		if (IS_ERR(pmic->rdev[id])) {
> +			ret = PTR_ERR(pmic->rdev[id]);
> +			dev_err(&pdev->dev, "Reg registeration %s failed: %d\n",

s/registeration/init/ (or registration?)

> +				rdesc->name, ret);
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int max77620_regulator_suspend(struct device *dev)
> +{
> +	struct max77620_regulator *pmic = dev_get_drvdata(dev);
> +	struct max77620_regulator_pdata *reg_pdata;
> +	struct max77620_regulator_info *rinfo;
> +	struct device *parent = pmic->dev->parent;
> +	int id;
> +	int ret;
> +
> +	for (id = 0; id < MAX77620_NUM_REGS; ++id) {
> +		reg_pdata = &pmic->reg_pdata[id];
> +		rinfo = pmic->rinfo[id];
> +		if (!reg_pdata || !rinfo)
> +			continue;
> +
> +		if (reg_pdata->disable_remote_sense_on_suspend &&
> +			(rinfo->remote_sense_addr != 0xFF)) {
> +			ret = max77620_reg_update(parent, MAX77620_PWR_SLAVE,
> +				rinfo->remote_sense_addr,
> +				rinfo->remote_sense_mask, 0);
> +			if (ret < 0)
> +				dev_err(dev, "Reg 0x%02x update failed: %d\n",
> +					rinfo->remote_sense_addr, ret);
> +		}
> +
> +		max77620_regulator_set_fps_slots(pmic, id, true);
> +		if (reg_pdata->suspend_fps_src >= 0)
> +			max77620_regulator_set_fps_src(pmic,
> +				reg_pdata->suspend_fps_src, id);
> +	}
> +
> +	return 0;
> +}
> +
> +static int max77620_regulator_resume(struct device *dev)
> +{
> +	struct max77620_regulator *pmic = dev_get_drvdata(dev);
> +	struct max77620_regulator_pdata *reg_pdata;
> +	struct max77620_regulator_info *rinfo;
> +	struct device *parent = pmic->dev->parent;
> +	int id;
> +	int ret;
> +
> +	for (id = 0; id < MAX77620_NUM_REGS; ++id) {
> +		reg_pdata = &pmic->reg_pdata[id];
> +		rinfo = pmic->rinfo[id];
> +		if (!reg_pdata || !rinfo)
> +			continue;
> +
> +		if (reg_pdata->disable_remote_sense_on_suspend &&
> +			(rinfo->remote_sense_addr != 0xFF)) {
> +			ret = max77620_reg_update(parent, MAX77620_PWR_SLAVE,
> +				rinfo->remote_sense_addr,
> +				rinfo->remote_sense_mask,
> +				rinfo->remote_sense_mask);
> +			if (ret < 0)
> +				dev_err(dev, "Reg 0x%02x update failed: %d\n",
> +					rinfo->remote_sense_addr, ret);
> +		}
> +
> +		max77620_regulator_set_fps_slots(pmic, id, false);
> +		if (reg_pdata->active_fps_src >= 0)
> +			max77620_regulator_set_fps_src(pmic,
> +				reg_pdata->active_fps_src, id);
> +	}
> +
> +	return 0;
> +}
> +#endif
> +
> +static const struct dev_pm_ops max77620_regulator_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(max77620_regulator_suspend,
> +			max77620_regulator_resume)
> +};
> +
> +static const struct platform_device_id max77620_regulator_devtype[] = {
> +	{
> +		.name = "max77620-pmic",
> +	},
> +	{
> +		.name = "max20024-pmic",
> +	},

Sentinel.
MODULE_DEVICE_TABLE

Best regards,
Krzysztof

[toc] | [prev] | [next] | [standalone]

#1308321 — Re: [PATCH V2 6/6] regulator: max77620: add regulator driver for max77620/max20024

[Multipart message — attachments visible in raw view] — view raw

On Wed, Jan 13, 2016 at 05:09:55PM +0530, Laxman Dewangan wrote:

> Yaah, this is something new and I think Mark also wanted to do this cleanup
> on his comment. I did not understand it previously. Looks cool now.

I did, please don't just ignore review comments.

> However, I have requirement to configure PMIC with some inital setting based
> on init_data (parsed) before regulator registration happened. Once I move to
> this new mecahinsm, the inti_data parsing and regulator init happen in the
> same function. So I need to have one callback from regulator_register to
> driver.
> Something below. This will do the pre configuration before
> regulator_register happena dn machine constraint sets.

> I can not use the init_data->regulator_init() as I need to have the init
> data passed to the driver.

Why do you have this requirement?

[toc] | [prev] | [next] | [standalone]

#1308344 — Re: [PATCH V2 6/6] regulator: max77620: add regulator driver for max77620/max20024

On Wednesday 13 January 2016 05:27 PM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Wed, Jan 13, 2016 at 05:09:55PM +0530, Laxman Dewangan wrote:
>
>> Yaah, this is something new and I think Mark also wanted to do this cleanup
>> on his comment. I did not understand it previously. Looks cool now.
> I did, please don't just ignore review comments.
Sure, I respect all comments and thanks for review.
Seems your mail got lost somewhere (still not seeing in my inbox).

>
>> However, I have requirement to configure PMIC with some inital setting based
>> on init_data (parsed) before regulator registration happened. Once I move to
>> this new mecahinsm, the inti_data parsing and regulator init happen in the
>> same function. So I need to have one callback from regulator_register to
>> driver.
>> Something below. This will do the pre configuration before
>> regulator_register happena dn machine constraint sets.
>> I can not use the init_data->regulator_init() as I need to have the init
>> data passed to the driver.
> Why do you have this requirement?

This is require to
- configure FPS_SRC based on platform data for each rail,
     - If FPS_SRC enabled then enable it always.
     - If FPS_SRC is disabled (NONE) then based on init data constraint,
       set it to desired state.
- Power mode and slew rate init based on default configuration from the 
register if platform does not want to set it.

[toc] | [prev] | [next] | [standalone]

#1308357 — Re: [PATCH V2 6/6] regulator: max77620: add regulator driver for max77620/max20024

[Multipart message — attachments visible in raw view] — view raw

On Wed, Jan 13, 2016 at 05:35:33PM +0530, Laxman Dewangan wrote:
> On Wednesday 13 January 2016 05:27 PM, Mark Brown wrote:

> >>Something below. This will do the pre configuration before
> >>regulator_register happena dn machine constraint sets.
> >>I can not use the init_data->regulator_init() as I need to have the init
> >>data passed to the driver.

> >Why do you have this requirement?

> This is require to
> - configure FPS_SRC based on platform data for each rail,
>     - If FPS_SRC enabled then enable it always.
>     - If FPS_SRC is disabled (NONE) then based on init data constraint,
>       set it to desired state.

What is FPS_SRC and why is it set from init_data?  A driver should never
be looking at init_data.

> - Power mode and slew rate init based on default configuration from the
> register if platform does not want to set it.

Same here, why is init_data needed here?

[toc] | [prev] | [next] | [standalone]

#1308369 — Re: [PATCH V2 6/6] regulator: max77620: add regulator driver for max77620/max20024

On Wednesday 13 January 2016 06:01 PM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Wed, Jan 13, 2016 at 05:35:33PM +0530, Laxman Dewangan wrote:
>> On Wednesday 13 January 2016 05:27 PM, Mark Brown wrote:
>> This is require to
>> - configure FPS_SRC based on platform data for each rail,
>>      - If FPS_SRC enabled then enable it always.
>>      - If FPS_SRC is disabled (NONE) then based on init data constraint,
>>        set it to desired state.
> What is FPS_SRC and why is it set from init_data?  A driver should never
> be looking at init_data.
FPS: Flexible Power Sequence.
Max77620 has three FPS source configuration FPS0,1 2 and can be 
configured with different digital input signals EN0, EN1 and EN2.
Then rails and few GPIOs can configured with these FPS SRC with 
different slots so that it can be controlled externally.

When FPS_SRC is set to NONE (this is needed from platform data) then 
based on constraint like boot enable/always enable, it need to be on 
desired state. Otherwise we may endup with disabling the rail when 
setting to NONE and create issue.

I need to set the FPS src properly for each rails before callback happen 
from regulator init so that enable/disable/is_enable can handle it properly.


>> - Power mode and slew rate init based on default configuration from the
>> register if platform does not want to set it.
> Same here, why is init_data needed here?
>
Here, init data is not needed. I was thinking to configure slew rate in 
preinit but can be done later also through regulator->ops.
But I will need the driver specific DT data (customized flag) to 
configure the PMIC like slots.

[toc] | [prev] | [next] | [standalone]

#1308382 — Re: [PATCH V2 6/6] regulator: max77620: add regulator driver for max77620/max20024

[Multipart message — attachments visible in raw view] — view raw

On Wed, Jan 13, 2016 at 06:07:51PM +0530, Laxman Dewangan wrote:
> On Wednesday 13 January 2016 06:01 PM, Mark Brown wrote:

> >What is FPS_SRC and why is it set from init_data?  A driver should never
> >be looking at init_data.

> When FPS_SRC is set to NONE (this is needed from platform data) then based
> on constraint like boot enable/always enable, it need to be on desired
> state. Otherwise we may endup with disabling the rail when setting to NONE
> and create issue.

What is "it" and why can't we check what the current configuration is
while setting FPS to NONE?

> I need to set the FPS src properly for each rails before callback happen
> from regulator init so that enable/disable/is_enable can handle it properly.

Why not just reorder the callback so it happens before the constraints
are applied?

[toc] | [prev] | [next] | [standalone]

#1308389 — Re: [PATCH V2 6/6] regulator: max77620: add regulator driver for max77620/max20024

On Wednesday 13 January 2016 06:39 PM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Wed, Jan 13, 2016 at 06:07:51PM +0530, Laxman Dewangan wrote:
>> On Wednesday 13 January 2016 06:01 PM, Mark Brown wrote:
>>> What is FPS_SRC and why is it set from init_data?  A driver should never
>>> be looking at init_data.
>> When FPS_SRC is set to NONE (this is needed from platform data) then based
>> on constraint like boot enable/always enable, it need to be on desired
>> state. Otherwise we may endup with disabling the rail when setting to NONE
>> and create issue.
> What is "it" and why can't we check what the current configuration is
> while setting FPS to NONE?
I will have platform specific data in callback of enable/disable only.
Which call back do I need to set this? enable/disable or post register?

>> I need to set the FPS src properly for each rails before callback happen
>> from regulator init so that enable/disable/is_enable can handle it properly.
> Why not just reorder the callback so it happens before the constraints
> are applied?
>
which callback I need to reorder for getting the context for configruation?

[toc] | [prev] | [next] | [standalone]

#1308450 — Re: [PATCH V2 6/6] regulator: max77620: add regulator driver for max77620/max20024

[Multipart message — attachments visible in raw view] — view raw

On Wed, Jan 13, 2016 at 06:41:11PM +0530, Laxman Dewangan wrote:
> On Wednesday 13 January 2016 06:39 PM, Mark Brown wrote:

> >What is "it" and why can't we check what the current configuration is
> >while setting FPS to NONE?

> I will have platform specific data in callback of enable/disable only.
> Which call back do I need to set this? enable/disable or post register?

I'm sorry but I can't parse what you're saying here.

> >>I need to set the FPS src properly for each rails before callback happen
> >>from regulator init so that enable/disable/is_enable can handle it properly.

> >Why not just reorder the callback so it happens before the constraints
> >are applied?

> which callback I need to reorder for getting the context for configruation?

The existing call to parse the DT.

[toc] | [prev] | [next] | [standalone]

#1308477 — Re: [PATCH V2 6/6] regulator: max77620: add regulator driver for max77620/max20024

On Wednesday 13 January 2016 07:50 PM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Wed, Jan 13, 2016 at 06:41:11PM +0530, Laxman Dewangan wrote:
>> On Wednesday 13 January 2016 06:39 PM, Mark Brown wrote:
>>> What is "it" and why can't we check what the current configuration is
>>> while setting FPS to NONE?
>> I will have platform specific data in callback of enable/disable only.
>> Which call back do I need to set this? enable/disable or post register?
> I'm sorry but I can't parse what you're saying here.
Here, before machine constraint applies, I need to configure FPS on 
desired source.
>>>> I need to set the FPS src properly for each rails before callback happen
>>> >from regulator init so that enable/disable/is_enable can handle it properly.
>>> Why not just reorder the callback so it happens before the constraints
>>> are applied?
>> which callback I need to reorder for getting the context for configruation?
> The existing call to parse the DT.

Are we allow to do i2c transfer in the existing callback for parsing the DT?
By code, nothing is stopping but want to know if there is such 
limitation on policy/design of this API.

[toc] | [prev] | [next] | [standalone]

#1308531 — Re: [PATCH V2 6/6] regulator: max77620: add regulator driver for max77620/max20024

[Multipart message — attachments visible in raw view] — view raw

On Wed, Jan 13, 2016 at 08:12:10PM +0530, Laxman Dewangan wrote:
> On Wednesday 13 January 2016 07:50 PM, Mark Brown wrote:
> >On Wed, Jan 13, 2016 at 06:41:11PM +0530, Laxman Dewangan wrote:
> >>On Wednesday 13 January 2016 06:39 PM, Mark Brown wrote:

> >>>What is "it" and why can't we check what the current configuration is
> >>>while setting FPS to NONE?

> >>I will have platform specific data in callback of enable/disable only.
> >>Which call back do I need to set this? enable/disable or post register?

> >I'm sorry but I can't parse what you're saying here.

> Here, before machine constraint applies, I need to configure FPS on desired
> source.

I'm a bit confused here - I had thought you needed things the other way
around?  Can you describe in concrete terms what you want to configure?

> >>>>I need to set the FPS src properly for each rails before callback happen
> >>>>from regulator init so that enable/disable/is_enable can handle it properly.

> >>>Why not just reorder the callback so it happens before the constraints
> >>>are applied?

> >>which callback I need to reorder for getting the context for configruation?
> >The existing call to parse the DT.

> Are we allow to do i2c transfer in the existing callback for parsing the DT?

Yes, of course.

[toc] | [prev] | [next] | [standalone]

#1308327 — Re: [PATCH V2 6/6] regulator: max77620: add regulator driver for max77620/max20024

On Wednesday 13 January 2016 06:58 AM, Krzysztof Kozlowski wrote:
> On 12.01.2016 18:17, Laxman Dewangan wrote:
> +
> +	ret = of_regulator_match(&pdev->dev, np, max77620_regulator_matches,
> +			ARRAY_SIZE(max77620_regulator_matches));
> Why not using 'regulators_node' and getting rid of this and some code
> below? If you need to parse custom regulator properties use 'of_parse_cb'.
>

Yaah, this is something new and I think Mark also wanted to do this 
cleanup on his comment. I did not understand it previously. Looks cool now.

However, I have requirement to configure PMIC with some inital setting 
based on init_data (parsed) before regulator registration happened. Once 
I move to this new mecahinsm, the inti_data parsing and regulator init 
happen in the same function. So I need to have one callback from 
regulator_register to driver.
Something below. This will do the pre configuration before 
regulator_register happena dn machine constraint sets.

I can not use the init_data->regulator_init() as I need to have the init 
data passed to the driver.

Created sample patch below.

ldewangan@ldewanganubuntu-System-Product-Name:~/upstream/linux-next/linux-next/drivers/regulator$ 
git diff core.c ../../include/linux/regulator/driver.h
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 3308c6b..aa2ca20 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3899,6 +3899,17 @@ regulator_register(const struct regulator_desc 
*regulator_desc,
                 rdev->dev.of_node = of_node_get(config->of_node);
         }

+       if (init_data && regulator_desc->pre_register_init) {
+               ret = regulator_desc->pre_register_init(regulator_desc,
+                                       config, init_data);
+               if (ret < 0) {
+                       dev_err(dev, "Pre register failed: %d\n", ret);
+                       kfree(rdev);
+                       kfree(config);
+                       return ERR_PTR(ret);
+               }
+       }
+
         mutex_lock(&regulator_list_mutex);

         mutex_init(&rdev->mutex);
diff --git a/include/linux/regulator/driver.h 
b/include/linux/regulator/driver.h
index 16ac9e1..fd1d989 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -280,6 +280,9 @@ struct regulator_desc {
         int (*of_parse_cb)(struct device_node *,
                             const struct regulator_desc *,
                             struct regulator_config *);
+       int (*pre_register_init)(const struct regulator_desc *,
+                       struct regulator_config *,
+                       struct regulator_init_data *);
         int id;
         bool continuous_voltage_range;
         unsigned n_voltages;

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web