[PATCH 0/4] regulator: Add support for configuration of active-discharge

[PATCH 0/4] regulator: Add support for configuration of active-discharge

[Laxman Dewangan]

This series add the common regulator DT property  for configuration
of active discharge and implementing this in regulator core.
Also implementing the regulator ops for MAX77620 to configure
active discharge.

This is based on discussion on patch and comment from Mark Brown:
[PATCH V6 7/8] DT: regulator: add DT binding doc for regulator of PMIC max77620/max20024

> +- maxim,disable-active-discharge:    boolean, Disable active discharge.

That's something I've seen other regulators do, can we make it a standard property with an op?


Laxman Dewangan (4):
  regulator: DT: Add DT property for active-discharge configuration
  regulator: core: Add support for active-discharge configuration
  regulator: helper: Add helper to configure active-discharge using
    regmap
  regulator: max77620: Add support to configure active-discharge

 .../devicetree/bindings/regulator/regulator.txt    |  5 +++++
 drivers/regulator/core.c                           | 10 ++++++++++
 drivers/regulator/helpers.c                        | 23 ++++++++++++++++++++++
 drivers/regulator/max77620-regulator.c             |  9 +++++++++
 drivers/regulator/of_regulator.c                   |  5 +++++
 include/linux/regulator/driver.h                   | 17 ++++++++++++++++
 include/linux/regulator/machine.h                  |  4 ++++
 7 files changed, 73 insertions(+)

-- 
2.1.4

[PATCH 0/4] regulator: Add support for configuration of active-discharge

[Laxman Dewangan]

This series add the common regulator DT property  for configuration
of active discharge and implementing this in regulator core.
Also implementing the regulator ops for MAX77620 to configure
active discharge.

This is based on discussion on patch and comment from Mark Brown:
[PATCH V6 7/8] DT: regulator: add DT binding doc for regulator of PMIC max77620/max20024

> +- maxim,disable-active-discharge:    boolean, Disable active discharge.

That's something I've seen other regulators do, can we make it a standard property with an op?


Laxman Dewangan (4):
  regulator: DT: Add DT property for active-discharge configuration
  regulator: core: Add support for active-discharge configuration
  regulator: helper: Add helper to configure active-discharge using
    regmap
  regulator: max77620: Add support to configure active-discharge

 .../devicetree/bindings/regulator/regulator.txt    |  5 +++++
 drivers/regulator/core.c                           | 10 ++++++++++
 drivers/regulator/helpers.c                        | 23 ++++++++++++++++++++++
 drivers/regulator/max77620-regulator.c             |  9 +++++++++
 drivers/regulator/of_regulator.c                   |  5 +++++
 include/linux/regulator/driver.h                   | 17 ++++++++++++++++
 include/linux/regulator/machine.h                  |  4 ++++
 7 files changed, 73 insertions(+)

-- 
2.1.4

[PATCH 1/4] regulator: DT: Add DT property for active-discharge configuration

[Laxman Dewangan]

Add common DT property for regulator node to support of
active discharge enable/disable configuration of regulator.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 Documentation/devicetree/bindings/regulator/regulator.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
index 1d112fc..ecfc593 100644
--- a/Documentation/devicetree/bindings/regulator/regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/regulator.txt
@@ -44,6 +44,11 @@ Optional properties:
   any consumer request.
 - regulator-pull-down: Enable pull down resistor when the regulator is disabled.
 - regulator-over-current-protection: Enable over current protection.
+- regulator-active-discharge: tristate, enable/disable active discharge of
+  regulators. The values are:
+	0: Disable active discharge.
+	1: Enable active discharge.
+	Absence of this property will leave configuration to default.
 
 Deprecated properties:
 - regulator-compatible: If a regulator chip contains multiple
-- 
2.1.4

[PATCH 1/4] regulator: DT: Add DT property for active-discharge configuration

[Laxman Dewangan]

Add common DT property for regulator node to support of
active discharge enable/disable configuration of regulator.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 Documentation/devicetree/bindings/regulator/regulator.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
index 1d112fc..ecfc593 100644
--- a/Documentation/devicetree/bindings/regulator/regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/regulator.txt
@@ -44,6 +44,11 @@ Optional properties:
   any consumer request.
 - regulator-pull-down: Enable pull down resistor when the regulator is disabled.
 - regulator-over-current-protection: Enable over current protection.
+- regulator-active-discharge: tristate, enable/disable active discharge of
+  regulators. The values are:
+	0: Disable active discharge.
+	1: Enable active discharge.
+	Absence of this property will leave configuration to default.
 
 Deprecated properties:
 - regulator-compatible: If a regulator chip contains multiple
-- 
2.1.4

[PATCH 2/4] regulator: core: Add support for active-discharge configuration

[Laxman Dewangan]

Add support to enable/disable active discharge of regulator via
machine constraints. This configuration is done when setting
machine constraint during regulator register and if regulator
driver implemented the callback ops.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/regulator/core.c          | 10 ++++++++++
 drivers/regulator/of_regulator.c  |  5 +++++
 include/linux/regulator/driver.h  |  3 +++
 include/linux/regulator/machine.h |  4 ++++
 4 files changed, 22 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 055f8c1..3e865b0 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1139,6 +1139,16 @@ static int set_machine_constraints(struct regulator_dev *rdev,
 		}
 	}
 
+	if ((rdev->constraints->active_discharge >= 0)
+		&& ops->set_active_discharge) {
+		ret = ops->set_active_discharge(rdev,
+				rdev->constraints->active_discharge);
+		if (ret < 0) {
+			rdev_err(rdev, "failed to set active discharge\n");
+			return ret;
+		}
+	}
+
 	print_constraints(rdev);
 	return 0;
 }
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 499e437..d1e0c03 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -93,6 +93,11 @@ static void of_get_regulation_constraints(struct device_node *np,
 
 	constraints->soft_start = of_property_read_bool(np,
 					"regulator-soft-start");
+	ret = of_property_read_u32(np, "regulator-active-discharge", &pval);
+	if (!ret)
+		constraints->active_discharge = pval;
+	else
+		constraints->active_discharge = -1;
 
 	if (!of_property_read_u32(np, "regulator-initial-mode", &pval)) {
 		if (desc && desc->of_map_mode) {
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 16ac9e1..59dbaf7 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -93,6 +93,8 @@ struct regulator_linear_range {
  * @get_current_limit: Get the configured limit for a current-limited regulator.
  * @set_input_current_limit: Configure an input limit.
  *
+ * @set_active_discharge: Set active discharge enable/disable of regulators.
+ *
  * @set_mode: Set the configured operating mode for the regulator.
  * @get_mode: Get the configured operating mode for the regulator.
  * @get_status: Return actual (not as-configured) status of regulator, as a
@@ -149,6 +151,7 @@ struct regulator_ops {
 
 	int (*set_input_current_limit) (struct regulator_dev *, int lim_uA);
 	int (*set_over_current_protection) (struct regulator_dev *);
+	int (*set_active_discharge) (struct regulator_dev *, bool enable);
 
 	/* enable/disable regulator */
 	int (*enable) (struct regulator_dev *);
diff --git a/include/linux/regulator/machine.h b/include/linux/regulator/machine.h
index a1067d0..408cde7 100644
--- a/include/linux/regulator/machine.h
+++ b/include/linux/regulator/machine.h
@@ -100,6 +100,8 @@ struct regulator_state {
  * @initial_state: Suspend state to set by default.
  * @initial_mode: Mode to set at startup.
  * @ramp_delay: Time to settle down after voltage change (unit: uV/us)
+ * @active_discharge: Enable/disable active discharge. The values are:
+ *		      -1: Default, 0: Disable, 1: Enable.
  * @enable_time: Turn-on time of the rails (unit: microseconds)
  */
 struct regulation_constraints {
@@ -140,6 +142,8 @@ struct regulation_constraints {
 	unsigned int ramp_delay;
 	unsigned int enable_time;
 
+	int active_discharge;
+
 	/* constraint flags */
 	unsigned always_on:1;	/* regulator never off when system is on */
 	unsigned boot_on:1;	/* bootloader/firmware enabled regulator */
-- 
2.1.4

[PATCH 2/4] regulator: core: Add support for active-discharge configuration

[Laxman Dewangan]

Add support to enable/disable active discharge of regulator via
machine constraints. This configuration is done when setting
machine constraint during regulator register and if regulator
driver implemented the callback ops.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/regulator/core.c          | 10 ++++++++++
 drivers/regulator/of_regulator.c  |  5 +++++
 include/linux/regulator/driver.h  |  3 +++
 include/linux/regulator/machine.h |  4 ++++
 4 files changed, 22 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 055f8c1..3e865b0 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1139,6 +1139,16 @@ static int set_machine_constraints(struct regulator_dev *rdev,
 		}
 	}
 
+	if ((rdev->constraints->active_discharge >= 0)
+		&& ops->set_active_discharge) {
+		ret = ops->set_active_discharge(rdev,
+				rdev->constraints->active_discharge);
+		if (ret < 0) {
+			rdev_err(rdev, "failed to set active discharge\n");
+			return ret;
+		}
+	}
+
 	print_constraints(rdev);
 	return 0;
 }
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 499e437..d1e0c03 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -93,6 +93,11 @@ static void of_get_regulation_constraints(struct device_node *np,
 
 	constraints->soft_start = of_property_read_bool(np,
 					"regulator-soft-start");
+	ret = of_property_read_u32(np, "regulator-active-discharge", &pval);
+	if (!ret)
+		constraints->active_discharge = pval;
+	else
+		constraints->active_discharge = -1;
 
 	if (!of_property_read_u32(np, "regulator-initial-mode", &pval)) {
 		if (desc && desc->of_map_mode) {
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 16ac9e1..59dbaf7 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -93,6 +93,8 @@ struct regulator_linear_range {
  * @get_current_limit: Get the configured limit for a current-limited regulator.
  * @set_input_current_limit: Configure an input limit.
  *
+ * @set_active_discharge: Set active discharge enable/disable of regulators.
+ *
  * @set_mode: Set the configured operating mode for the regulator.
  * @get_mode: Get the configured operating mode for the regulator.
  * @get_status: Return actual (not as-configured) status of regulator, as a
@@ -149,6 +151,7 @@ struct regulator_ops {
 
 	int (*set_input_current_limit) (struct regulator_dev *, int lim_uA);
 	int (*set_over_current_protection) (struct regulator_dev *);
+	int (*set_active_discharge) (struct regulator_dev *, bool enable);
 
 	/* enable/disable regulator */
 	int (*enable) (struct regulator_dev *);
diff --git a/include/linux/regulator/machine.h b/include/linux/regulator/machine.h
index a1067d0..408cde7 100644
--- a/include/linux/regulator/machine.h
+++ b/include/linux/regulator/machine.h
@@ -100,6 +100,8 @@ struct regulator_state {
  * @initial_state: Suspend state to set by default.
  * @initial_mode: Mode to set at startup.
  * @ramp_delay: Time to settle down after voltage change (unit: uV/us)
+ * @active_discharge: Enable/disable active discharge. The values are:
+ *		      -1: Default, 0: Disable, 1: Enable.
  * @enable_time: Turn-on time of the rails (unit: microseconds)
  */
 struct regulation_constraints {
@@ -140,6 +142,8 @@ struct regulation_constraints {
 	unsigned int ramp_delay;
 	unsigned int enable_time;
 
+	int active_discharge;
+
 	/* constraint flags */
 	unsigned always_on:1;	/* regulator never off when system is on */
 	unsigned boot_on:1;	/* bootloader/firmware enabled regulator */
-- 
2.1.4

[PATCH 3/4] regulator: helper: Add helper to configure active-discharge using regmap

[Laxman Dewangan]

Add helper function to set the state of active-discharge of
regulator using regmap. The HW regulator driver can directly
use this by providing the necessary information in the regulator
descriptor.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/regulator/helpers.c      | 23 +++++++++++++++++++++++
 include/linux/regulator/driver.h | 14 ++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/drivers/regulator/helpers.c b/drivers/regulator/helpers.c
index 3bbb326..b1e32e7 100644
--- a/drivers/regulator/helpers.c
+++ b/drivers/regulator/helpers.c
@@ -465,3 +465,26 @@ int regulator_get_bypass_regmap(struct regulator_dev *rdev, bool *enable)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(regulator_get_bypass_regmap);
+
+/**
+ * regulator_set_active_discharge_regmap - Default set_active_discharge()
+ *					   using regmap
+ *
+ * @rdev: device to operate on.
+ * @enable: state to set, 0 to disable and 1 to enable.
+ */
+int regulator_set_active_discharge_regmap(struct regulator_dev *rdev,
+					  bool enable)
+{
+	unsigned int val;
+
+	if (enable)
+		val = rdev->desc->active_discharge_on;
+	else
+		val = rdev->desc->active_discharge_off;
+
+	return regmap_update_bits(rdev->regmap,
+				  rdev->desc->active_discharge_reg,
+				  rdev->desc->active_discharge_mask, val);
+}
+EXPORT_SYMBOL_GPL(regulator_set_active_discharge_regmap);
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 59dbaf7..cd271e8 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -269,6 +269,14 @@ enum regulator_type {
  * @bypass_mask: Mask for control when using regmap set_bypass
  * @bypass_val_on: Enabling value for control when using regmap set_bypass
  * @bypass_val_off: Disabling value for control when using regmap set_bypass
+ * @active_discharge_off: Enabling value for control when using regmap
+ *			  set_active_discharge
+ * @active_discharge_on: Disabling value for control when using regmap
+ *			 set_active_discharge
+ * @active_discharge_mask: Mask for control when using regmap
+ *			   set_active_discharge
+ * @active_discharge_reg: Register for control when using regmap
+ *			  set_active_discharge
  *
  * @enable_time: Time taken for initial enable of regulator (in uS).
  * @off_on_delay: guard time (in uS), before re-enabling a regulator
@@ -318,6 +326,10 @@ struct regulator_desc {
 	unsigned int bypass_mask;
 	unsigned int bypass_val_on;
 	unsigned int bypass_val_off;
+	unsigned int active_discharge_on;
+	unsigned int active_discharge_off;
+	unsigned int active_discharge_mask;
+	unsigned int active_discharge_reg;
 
 	unsigned int enable_time;
 
@@ -450,6 +462,8 @@ int regulator_set_voltage_time_sel(struct regulator_dev *rdev,
 int regulator_set_bypass_regmap(struct regulator_dev *rdev, bool enable);
 int regulator_get_bypass_regmap(struct regulator_dev *rdev, bool *enable);
 
+int regulator_set_active_discharge_regmap(struct regulator_dev *rdev,
+					  bool enable);
 void *regulator_get_init_drvdata(struct regulator_init_data *reg_init_data);
 
 #endif
-- 
2.1.4

[PATCH 3/4] regulator: helper: Add helper to configure active-discharge using regmap

[Laxman Dewangan]

Add helper function to set the state of active-discharge of
regulator using regmap. The HW regulator driver can directly
use this by providing the necessary information in the regulator
descriptor.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/regulator/helpers.c      | 23 +++++++++++++++++++++++
 include/linux/regulator/driver.h | 14 ++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/drivers/regulator/helpers.c b/drivers/regulator/helpers.c
index 3bbb326..b1e32e7 100644
--- a/drivers/regulator/helpers.c
+++ b/drivers/regulator/helpers.c
@@ -465,3 +465,26 @@ int regulator_get_bypass_regmap(struct regulator_dev *rdev, bool *enable)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(regulator_get_bypass_regmap);
+
+/**
+ * regulator_set_active_discharge_regmap - Default set_active_discharge()
+ *					   using regmap
+ *
+ * @rdev: device to operate on.
+ * @enable: state to set, 0 to disable and 1 to enable.
+ */
+int regulator_set_active_discharge_regmap(struct regulator_dev *rdev,
+					  bool enable)
+{
+	unsigned int val;
+
+	if (enable)
+		val = rdev->desc->active_discharge_on;
+	else
+		val = rdev->desc->active_discharge_off;
+
+	return regmap_update_bits(rdev->regmap,
+				  rdev->desc->active_discharge_reg,
+				  rdev->desc->active_discharge_mask, val);
+}
+EXPORT_SYMBOL_GPL(regulator_set_active_discharge_regmap);
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 59dbaf7..cd271e8 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -269,6 +269,14 @@ enum regulator_type {
  * @bypass_mask: Mask for control when using regmap set_bypass
  * @bypass_val_on: Enabling value for control when using regmap set_bypass
  * @bypass_val_off: Disabling value for control when using regmap set_bypass
+ * @active_discharge_off: Enabling value for control when using regmap
+ *			  set_active_discharge
+ * @active_discharge_on: Disabling value for control when using regmap
+ *			 set_active_discharge
+ * @active_discharge_mask: Mask for control when using regmap
+ *			   set_active_discharge
+ * @active_discharge_reg: Register for control when using regmap
+ *			  set_active_discharge
  *
  * @enable_time: Time taken for initial enable of regulator (in uS).
  * @off_on_delay: guard time (in uS), before re-enabling a regulator
@@ -318,6 +326,10 @@ struct regulator_desc {
 	unsigned int bypass_mask;
 	unsigned int bypass_val_on;
 	unsigned int bypass_val_off;
+	unsigned int active_discharge_on;
+	unsigned int active_discharge_off;
+	unsigned int active_discharge_mask;
+	unsigned int active_discharge_reg;
 
 	unsigned int enable_time;
 
@@ -450,6 +462,8 @@ int regulator_set_voltage_time_sel(struct regulator_dev *rdev,
 int regulator_set_bypass_regmap(struct regulator_dev *rdev, bool enable);
 int regulator_get_bypass_regmap(struct regulator_dev *rdev, bool *enable);
 
+int regulator_set_active_discharge_regmap(struct regulator_dev *rdev,
+					  bool enable);
 void *regulator_get_init_drvdata(struct regulator_init_data *reg_init_data);
 
 #endif
-- 
2.1.4

[PATCH 4/4] regulator: max77620: Add support to configure active-discharge

[Laxman Dewangan]

Add regulator ops callback for configuration of active-discharge
and provide necessarily information via regulator descriptor.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/regulator/max77620-regulator.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/regulator/max77620-regulator.c b/drivers/regulator/max77620-regulator.c
index 259e7e1..73a3356 100644
--- a/drivers/regulator/max77620-regulator.c
+++ b/drivers/regulator/max77620-regulator.c
@@ -578,6 +578,7 @@ static struct regulator_ops max77620_regulator_ops = {
 	.get_mode = max77620_regulator_get_mode,
 	.set_ramp_delay = max77620_regulator_set_ramp_delay,
 	.set_voltage_time_sel = regulator_set_voltage_time_sel,
+	.set_active_discharge = regulator_set_active_discharge_regmap,
 };
 
 #define MAX77620_SD_CNF2_ROVS_EN_NONE	0
@@ -606,6 +607,10 @@ static struct regulator_ops max77620_regulator_ops = {
 			.enable_time = 500,				\
 			.vsel_mask = MAX77620_##_volt_mask##_VOLT_MASK,	\
 			.vsel_reg = MAX77620_REG_##_id,			\
+			.active_discharge_off = 0,			\
+			.active_discharge_on = MAX77620_SD_CFG1_ADE_ENABLE, \
+			.active_discharge_mask = MAX77620_SD_CFG1_ADE_MASK, \
+			.active_discharge_reg = MAX77620_REG_##_id##_CFG, \
 			.type = REGULATOR_VOLTAGE,			\
 		},							\
 	}
@@ -633,6 +638,10 @@ static struct regulator_ops max77620_regulator_ops = {
 			.enable_time = 500,				\
 			.vsel_mask = MAX77620_LDO_VOLT_MASK,		\
 			.vsel_reg = MAX77620_REG_##_id##_CFG,		\
+			.active_discharge_off = 0,			\
+			.active_discharge_on = MAX77620_LDO_CFG2_ADE_ENABLE, \
+			.active_discharge_mask = MAX77620_LDO_CFG2_ADE_MASK, \
+			.active_discharge_reg = MAX77620_REG_##_id##_CFG2, \
 			.type = REGULATOR_VOLTAGE,			\
 		},							\
 	}
-- 
2.1.4

[PATCH 4/4] regulator: max77620: Add support to configure active-discharge

[Laxman Dewangan]

Add regulator ops callback for configuration of active-discharge
and provide necessarily information via regulator descriptor.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/regulator/max77620-regulator.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/regulator/max77620-regulator.c b/drivers/regulator/max77620-regulator.c
index 259e7e1..73a3356 100644
--- a/drivers/regulator/max77620-regulator.c
+++ b/drivers/regulator/max77620-regulator.c
@@ -578,6 +578,7 @@ static struct regulator_ops max77620_regulator_ops = {
 	.get_mode = max77620_regulator_get_mode,
 	.set_ramp_delay = max77620_regulator_set_ramp_delay,
 	.set_voltage_time_sel = regulator_set_voltage_time_sel,
+	.set_active_discharge = regulator_set_active_discharge_regmap,
 };
 
 #define MAX77620_SD_CNF2_ROVS_EN_NONE	0
@@ -606,6 +607,10 @@ static struct regulator_ops max77620_regulator_ops = {
 			.enable_time = 500,				\
 			.vsel_mask = MAX77620_##_volt_mask##_VOLT_MASK,	\
 			.vsel_reg = MAX77620_REG_##_id,			\
+			.active_discharge_off = 0,			\
+			.active_discharge_on = MAX77620_SD_CFG1_ADE_ENABLE, \
+			.active_discharge_mask = MAX77620_SD_CFG1_ADE_MASK, \
+			.active_discharge_reg = MAX77620_REG_##_id##_CFG, \
 			.type = REGULATOR_VOLTAGE,			\
 		},							\
 	}
@@ -633,6 +638,10 @@ static struct regulator_ops max77620_regulator_ops = {
 			.enable_time = 500,				\
 			.vsel_mask = MAX77620_LDO_VOLT_MASK,		\
 			.vsel_reg = MAX77620_REG_##_id##_CFG,		\
+			.active_discharge_off = 0,			\
+			.active_discharge_on = MAX77620_LDO_CFG2_ADE_ENABLE, \
+			.active_discharge_mask = MAX77620_LDO_CFG2_ADE_MASK, \
+			.active_discharge_reg = MAX77620_REG_##_id##_CFG2, \
 			.type = REGULATOR_VOLTAGE,			\
 		},							\
 	}
-- 
2.1.4

Re: [PATCH 2/4] regulator: core: Add support for active-discharge configuration

[Mark Brown]

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

On Tue, Mar 01, 2016 at 06:08:13PM +0530, Laxman Dewangan wrote:

> + * @active_discharge: Enable/disable active discharge. The values are:
> + *		      -1: Default, 0: Disable, 1: Enable.

This isn't a good interface, it means that 0 (which is the default value
for static constrants or kzalloc()ed constraints) means to actively
discharge rather than leave the settings unchanged but our general
policy is to not touch anything unless explicitly told to do so.

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

Re: [PATCH 2/4] regulator: core: Add support for active-discharge configuration

[Laxman Dewangan]

On Wednesday 02 March 2016 07:14 AM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Tue, Mar 01, 2016 at 06:08:13PM +0530, Laxman Dewangan wrote:
>
>> + * @active_discharge: Enable/disable active discharge. The values are:
>> + *		      -1: Default, 0: Disable, 1: Enable.
> This isn't a good interface, it means that 0 (which is the default value
> for static constrants or kzalloc()ed constraints) means to actively
> discharge rather than leave the settings unchanged but our general
> policy is to not touch anything unless explicitly told to do so.
>
Yaah, I agree.
I had other option to use 0 as default, 1 as disable and 2 as enable. 
but generally for disable, we use 0 and for enable 1 and that's why I 
went to -1,0,1 approach.

Will it be fine to have this as
   0: Default i..e do not change.
   1: disable.
   2: Enable.

Re: [PATCH 2/4] regulator: core: Add support for active-discharge configuration

[Laxman Dewangan]

On Wednesday 02 March 2016 07:14 AM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Tue, Mar 01, 2016 at 06:08:13PM +0530, Laxman Dewangan wrote:
>
>> + * @active_discharge: Enable/disable active discharge. The values are:
>> + *		      -1: Default, 0: Disable, 1: Enable.
> This isn't a good interface, it means that 0 (which is the default value
> for static constrants or kzalloc()ed constraints) means to actively
> discharge rather than leave the settings unchanged but our general
> policy is to not touch anything unless explicitly told to do so.
>
Yaah, I agree.
I had other option to use 0 as default, 1 as disable and 2 as enable. 
but generally for disable, we use 0 and for enable 1 and that's why I 
went to -1,0,1 approach.

Will it be fine to have this as
   0: Default i..e do not change.
   1: disable.
   2: Enable.

Re: [PATCH 2/4] regulator: core: Add support for active-discharge configuration

[Mark Brown]

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

On Wed, Mar 02, 2016 at 07:31:40AM +0530, Laxman Dewangan wrote:

> Yaah, I agree.
> I had other option to use 0 as default, 1 as disable and 2 as enable. but
> generally for disable, we use 0 and for enable 1 and that's why I went to
> -1,0,1 approach.

> Will it be fine to have this as
>   0: Default i..e do not change.
>   1: disable.
>   2: Enable.

I'd suggest defining an enum here with default explicitly set to 0 but
yes.

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

Re: [PATCH 2/4] regulator: core: Add support for active-discharge configuration

[Mark Brown]

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

On Wed, Mar 02, 2016 at 07:31:40AM +0530, Laxman Dewangan wrote:

> Yaah, I agree.
> I had other option to use 0 as default, 1 as disable and 2 as enable. but
> generally for disable, we use 0 and for enable 1 and that's why I went to
> -1,0,1 approach.

> Will it be fine to have this as
>   0: Default i..e do not change.
>   1: disable.
>   2: Enable.

I'd suggest defining an enum here with default explicitly set to 0 but
yes.

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