[PATCH v3 0/3] bd718x7: Support SNVS low power state

[PATCH v3 0/3] bd718x7: Support SNVS low power state

[Matti Vaittinen]

Patch series adding SNVS power state support to ROHM bd718x7 driver.

The SNVS is a low power state used by i.MX family of SoCs. In SNVS
state processor and most of the peripherials are shut off in order
to minimize power consumption.

BD71837 and BD71847 can be configured to use the SNVS state as a
reset/shutdown target state. There is some HW limitations though.
Main limitation is that regulators which have been under SW control
(regarding enable/disable state) will stay shut down after such reset.
Thus if SNVS is used as reset target the control of boot crucial
regulators must be left to HW state machine.

This patch seris adds DT properties for specifying whether to use
SNVS as reset target state and for setting the HW run level
specific voltages.

Changelog v3:
Patch 2 unchanged
- DT bindings SNVS rest description improved(?)
- Added optional HW state properties in usage example
- Fixed a bug from DVS voltage settings (buck 1 was initialize with
  voltag from buck 2 and buck 2 was uninitialized) - thanks to Angus
  Ainslie for reoporting.

Changelog v2:
- drop RFC tag
- add handling of SNVS as reset target
- add skipping SW control for critical regulators based on presense
  of regulator-always-on and regulator-boot-on

---

Matti Vaittinen (3):
  devicetree: bindings: bd718x7: document HW state related ROHM specific
    properties
  regulator: add regulator_desc_list_voltage_linear_range
  regulator: bd718x7: Support SNVS low power state

 .../devicetree/bindings/mfd/rohm,bd71837-pmic.txt  |  17 ++
 .../bindings/regulator/rohm,bd71837-regulator.txt  |  38 ++++
 drivers/regulator/bd718x7-regulator.c              | 201 ++++++++++++++++++---
 drivers/regulator/helpers.c                        |  39 +++-
 include/linux/regulator/driver.h                   |   6 +
 5 files changed, 266 insertions(+), 35 deletions(-)

-- 
2.14.3


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes.  Just then, he vanished ~~~

[PATCH v3 1/3] devicetree: bindings: bd718x7: document HW state related ROHM specific properties

[Matti Vaittinen]

Add ROHM BD71837 / BD71847 specific device tree bindings for
controlling the PMIC shutdown/reset states and voltages for
different HW states. The PMIC was designed to be used with NXP
i.MX8 SoC and it supports SNVS low power state which seems to
be typical for NXP i.MX SoCs. However, when SNVS is used we must
not allow SW to control enabling/disabling those regulators which
are crucial for system to boot as there is a HW limitation which
causes SW controlled regulators to be kept shut down after SNVS
reset.

Allow setting the SNVS to be used as reset target state and allow
marking those regulators which are critical for boot.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
 .../devicetree/bindings/mfd/rohm,bd71837-pmic.txt  | 17 ++++++++++
 .../bindings/regulator/rohm,bd71837-regulator.txt  | 38 ++++++++++++++++++++++
 2 files changed, 55 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt b/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
index a4b056761eaa..d5f68ac78d15 100644
--- a/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
+++ b/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
@@ -23,6 +23,20 @@ Required properties:
 
 Optional properties:
 - clock-output-names	: Should contain name for output clock.
+- rohm,reset-snvs-powered : Transfer BD718x7 to SNVS state at reset.
+
+The BD718x7 supports two different HW states as reset target states. States
+are called as SNVS and READY. At READY state all the PMIC power outputs go
+down and OTP is reload. At the SNVS state all other logic and external
+devices apart from the SNVS power domain are shut off. Please refer to NXP
+i.MX8 documentation for further information regarding SNVS state. When a
+reset is done via SNVS state the PMIC OTP data is not reload. This causes
+power outputs that have been under SW control to stay down when reset has
+switched power state to SNVS. If reset is done via READY state the power
+outputs will be returned to HW control by OTP loading. Thus the reset
+target state is set to READY by default. If SNVS state is used the boot
+crucial regulators must have the regulator-always-on and regulator-boot-on
+properties set in regulator node.
 
 Example:
 
@@ -43,6 +57,7 @@ Example:
 		#clock-cells = <0>;
 		clocks = <&osc 0>;
 		clock-output-names = "bd71837-32k-out";
+		rohm,reset-snvs-powered;
 
 		regulators {
 			buck1: BUCK1 {
@@ -50,8 +65,10 @@ Example:
 				regulator-min-microvolt = <700000>;
 				regulator-max-microvolt = <1300000>;
 				regulator-boot-on;
+				regulator-always-on;
 				regulator-ramp-delay = <1250>;
 			};
+			// [...]
 		};
 	};
 
diff --git a/Documentation/devicetree/bindings/regulator/rohm,bd71837-regulator.txt b/Documentation/devicetree/bindings/regulator/rohm,bd71837-regulator.txt
index 4b98ca26e61a..cbce62c22b60 100644
--- a/Documentation/devicetree/bindings/regulator/rohm,bd71837-regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/rohm,bd71837-regulator.txt
@@ -27,8 +27,38 @@ BUCK1, BUCK2, BUCK3, BUCK4, BUCK5, BUCK6
 LDO1, LDO2, LDO3, LDO4, LDO5, LDO6
 
 Optional properties:
+- rohm,dvs-run-voltage		: PMIC default "RUN" state voltage in uV.
+				  See below table for bucks which support this.
+- rohm,dvs-idle-voltage		: PMIC default "IDLE" state voltage in uV.
+				  See below table for bucks which support this.
+- rohm,dvs-suspend-voltage	: PMIC default "SUSPEND" state voltage in uV.
+				  See below table for bucks which support this.
 - Any optional property defined in bindings/regulator/regulator.txt
 
+Supported default DVS states:
+
+BD71837:
+buck	| dvs-run-voltage	| dvs-idle-voltage	| dvs-suspend-voltage
+-----------------------------------------------------------------------------
+1	| supported		| supported		| supported
+----------------------------------------------------------------------------
+2	| supported		| supported		| not supported
+----------------------------------------------------------------------------
+3	| supported		| not supported		| not supported
+----------------------------------------------------------------------------
+4	| supported		| not supported		| not supported
+----------------------------------------------------------------------------
+rest	| not supported		| not supported		| not supported
+
+BD71847:
+buck	| dvs-run-voltage	| dvs-idle-voltage	| dvs-suspend-voltage
+-----------------------------------------------------------------------------
+1	| supported		| supported		| supported
+----------------------------------------------------------------------------
+2	| supported		| supported		| not supported
+----------------------------------------------------------------------------
+rest	| not supported		| not supported		| not supported
+
 Example:
 regulators {
 	buck1: BUCK1 {
@@ -36,7 +66,11 @@ regulators {
 		regulator-min-microvolt = <700000>;
 		regulator-max-microvolt = <1300000>;
 		regulator-boot-on;
+		regulator-always-on;
 		regulator-ramp-delay = <1250>;
+		rohm,dvs-run-voltage = <900000>;
+		rohm,dvs-idle-voltage = <850000>;
+		rohm,dvs-suspend-voltage = <800000>;
 	};
 	buck2: BUCK2 {
 		regulator-name = "buck2";
@@ -45,18 +79,22 @@ regulators {
 		regulator-boot-on;
 		regulator-always-on;
 		regulator-ramp-delay = <1250>;
+		rohm,dvs-run-voltage = <1000000>;
+		rohm,dvs-idle-voltage = <900000>;
 	};
 	buck3: BUCK3 {
 		regulator-name = "buck3";
 		regulator-min-microvolt = <700000>;
 		regulator-max-microvolt = <1300000>;
 		regulator-boot-on;
+		rohm,dvs-run-voltage = <1000000>;
 	};
 	buck4: BUCK4 {
 		regulator-name = "buck4";
 		regulator-min-microvolt = <700000>;
 		regulator-max-microvolt = <1300000>;
 		regulator-boot-on;
+		rohm,dvs-run-voltage = <1000000>;
 	};
 	buck5: BUCK5 {
 		regulator-name = "buck5";
-- 
2.14.3


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes.  Just then, he vanished ~~~

[PATCH v3 2/3] regulator: add regulator_desc_list_voltage_linear_range

[Matti Vaittinen]

Add regulator_desc_list_voltage_linear_range which can be used
by drivers for getting the voltages before regulator is registered.
This may be useful for drivers which need to fetch the voltage
selectors at device-tree parsing callback.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Acked-by: Mark Brown <broonie@kernel.org>
---

Mark, I converted your:
"This seems fine." from RFC patch v1 into acked-by. This should
be unchanged. Please let me know if that's not Ok.

 drivers/regulator/helpers.c      | 39 +++++++++++++++++++++++++++++----------
 include/linux/regulator/driver.h |  6 ++++++
 2 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/drivers/regulator/helpers.c b/drivers/regulator/helpers.c
index 5686a1335bd3..68ac6017ef28 100644
--- a/drivers/regulator/helpers.c
+++ b/drivers/regulator/helpers.c
@@ -594,28 +594,30 @@ int regulator_list_voltage_pickable_linear_range(struct regulator_dev *rdev,
 EXPORT_SYMBOL_GPL(regulator_list_voltage_pickable_linear_range);
 
 /**
- * regulator_list_voltage_linear_range - List voltages for linear ranges
+ * regulator_desc_list_voltage_linear_range - List voltages for linear ranges
  *
- * @rdev: Regulator device
+ * @desc: Regulator desc for regulator which volatges are to be listed
  * @selector: Selector to convert into a voltage
  *
  * Regulators with a series of simple linear mappings between voltages
- * and selectors can set linear_ranges in the regulator descriptor and
- * then use this function as their list_voltage() operation,
+ * and selectors who have set linear_ranges in the regulator descriptor
+ * can use this function prior regulator registration to list voltages.
+ * This is useful when voltages need to be listed during device-tree
+ * parsing.
  */
-int regulator_list_voltage_linear_range(struct regulator_dev *rdev,
-					unsigned int selector)
+int regulator_desc_list_voltage_linear_range(const struct regulator_desc *desc,
+					     unsigned int selector)
 {
 	const struct regulator_linear_range *range;
 	int i;
 
-	if (!rdev->desc->n_linear_ranges) {
-		BUG_ON(!rdev->desc->n_linear_ranges);
+	if (!desc->n_linear_ranges) {
+		BUG_ON(!desc->n_linear_ranges);
 		return -EINVAL;
 	}
 
-	for (i = 0; i < rdev->desc->n_linear_ranges; i++) {
-		range = &rdev->desc->linear_ranges[i];
+	for (i = 0; i < desc->n_linear_ranges; i++) {
+		range = &desc->linear_ranges[i];
 
 		if (!(selector >= range->min_sel &&
 		      selector <= range->max_sel))
@@ -628,6 +630,23 @@ int regulator_list_voltage_linear_range(struct regulator_dev *rdev,
 
 	return -EINVAL;
 }
+EXPORT_SYMBOL_GPL(regulator_desc_list_voltage_linear_range);
+
+/**
+ * regulator_list_voltage_linear_range - List voltages for linear ranges
+ *
+ * @rdev: Regulator device
+ * @selector: Selector to convert into a voltage
+ *
+ * Regulators with a series of simple linear mappings between voltages
+ * and selectors can set linear_ranges in the regulator descriptor and
+ * then use this function as their list_voltage() operation,
+ */
+int regulator_list_voltage_linear_range(struct regulator_dev *rdev,
+					unsigned int selector)
+{
+	return regulator_desc_list_voltage_linear_range(rdev->desc, selector);
+}
 EXPORT_SYMBOL_GPL(regulator_list_voltage_linear_range);
 
 /**
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 7f8345bff4e1..05efe2b057c1 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -539,4 +539,10 @@ void *regulator_get_init_drvdata(struct regulator_init_data *reg_init_data);
 void regulator_lock(struct regulator_dev *rdev);
 void regulator_unlock(struct regulator_dev *rdev);
 
+/*
+ * Helper functions intended to be used by regulator drivers prior registering
+ * their regulators.
+ */
+int regulator_desc_list_voltage_linear_range(const struct regulator_desc *desc,
+					     unsigned int selector);
 #endif
-- 
2.14.3


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes.  Just then, he vanished ~~~

[PATCH v3 3/3] regulator: bd718x7: Support SNVS low power state

[Matti Vaittinen]

read ROHM BD71837 / BD71847 specific device tree bindings for
controlling the PMIC shutdown/reset states and voltages for
different HW states. The PMIC was designed to be used with NXP
i.MX8 SoC and it supports SNVS low power state which seems to
be typical for NXP i.MX SoCs. However, when SNVS is used we must
not allow SW to control enabling/disabling those regulators which
are crucial for system to boot as there is a HW limitation which
causes SW controlled regulators to be kept shut down after SNVS
reset.

Allow setting the SNVS to be used as reset target state and allow
marking those regulators which are critical for boot.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---

Angus, I would be grateful if you have a chance to test this
on your system :)

 drivers/regulator/bd718x7-regulator.c | 201 +++++++++++++++++++++++++++++-----
 1 file changed, 176 insertions(+), 25 deletions(-)

diff --git a/drivers/regulator/bd718x7-regulator.c b/drivers/regulator/bd718x7-regulator.c
index ccea133778c8..b2191be49670 100644
--- a/drivers/regulator/bd718x7-regulator.c
+++ b/drivers/regulator/bd718x7-regulator.c
@@ -350,6 +350,135 @@ static const struct reg_init bd71837_ldo6_inits[] = {
 	},
 };
 
+#define NUM_DVS_BUCKS 4
+
+struct of_dvs_setting {
+	const char *prop;
+	unsigned int reg;
+};
+
+static int set_dvs_levels(const struct of_dvs_setting *dvs,
+			  struct device_node *np,
+			  const struct regulator_desc *desc,
+			  struct regmap *regmap)
+{
+	int ret, i;
+	unsigned int uv;
+
+	ret = of_property_read_u32(np, dvs->prop, &uv);
+	if (ret) {
+		if (ret != -EINVAL)
+			return ret;
+		return 0;
+	}
+
+	for (i = 0; i < desc->n_voltages; i++) {
+		ret = regulator_desc_list_voltage_linear_range(desc, i);
+		if (ret < 0)
+			continue;
+		if (ret == uv) {
+			i <<= ffs(desc->vsel_mask) - 1;
+			ret = regmap_update_bits(regmap, dvs->reg,
+						 DVS_BUCK_RUN_MASK, i);
+			break;
+		}
+	}
+	return ret;
+}
+
+static int buck4_set_hw_dvs_levels(struct device_node *np,
+			    const struct regulator_desc *desc,
+			    struct regulator_config *cfg)
+{
+	int ret, i;
+	const struct of_dvs_setting dvs[] = {
+		{
+			.prop = "rohm,dvs-run-voltage",
+			.reg = BD71837_REG_BUCK4_VOLT_RUN,
+		},
+	};
+
+	for (i = 0; i < ARRAY_SIZE(dvs); i++) {
+		ret = set_dvs_levels(&dvs[i], np, desc, cfg->regmap);
+		if (ret)
+			break;
+	}
+	return ret;
+}
+static int buck3_set_hw_dvs_levels(struct device_node *np,
+			    const struct regulator_desc *desc,
+			    struct regulator_config *cfg)
+{
+	int ret, i;
+	const struct of_dvs_setting dvs[] = {
+		{
+			.prop = "rohm,dvs-run-voltage",
+			.reg = BD71837_REG_BUCK3_VOLT_RUN,
+		},
+	};
+
+	for (i = 0; i < ARRAY_SIZE(dvs); i++) {
+		ret = set_dvs_levels(&dvs[i], np, desc, cfg->regmap);
+		if (ret)
+			break;
+	}
+	return ret;
+}
+
+static int buck2_set_hw_dvs_levels(struct device_node *np,
+			    const struct regulator_desc *desc,
+			    struct regulator_config *cfg)
+{
+	int ret, i;
+	const struct of_dvs_setting dvs[] = {
+		{
+			.prop = "rohm,dvs-run-voltage",
+			.reg = BD718XX_REG_BUCK2_VOLT_RUN,
+		},
+		{
+			.prop = "rohm,dvs-idle-voltage",
+			.reg = BD718XX_REG_BUCK2_VOLT_IDLE,
+		},
+	};
+
+
+
+	for (i = 0; i < ARRAY_SIZE(dvs); i++) {
+		ret = set_dvs_levels(&dvs[i], np, desc, cfg->regmap);
+		if (ret)
+			break;
+	}
+	return ret;
+}
+
+static int buck1_set_hw_dvs_levels(struct device_node *np,
+			    const struct regulator_desc *desc,
+			    struct regulator_config *cfg)
+{
+	int ret, i;
+	const struct of_dvs_setting dvs[] = {
+		{
+			.prop = "rohm,dvs-run-voltage",
+			.reg = BD718XX_REG_BUCK1_VOLT_RUN,
+		},
+		{
+			.prop = "rohm,dvs-idle-voltage",
+			.reg = BD718XX_REG_BUCK1_VOLT_IDLE,
+		},
+		{
+			.prop = "rohm,dvs-suspend-voltage",
+			.reg = BD718XX_REG_BUCK1_VOLT_SUSP,
+		},
+	};
+
+	for (i = 0; i < ARRAY_SIZE(dvs); i++) {
+		ret = set_dvs_levels(&dvs[i], np, desc, cfg->regmap);
+		if (ret)
+			break;
+	}
+	return ret;
+}
+
 static const struct bd718xx_regulator_data bd71847_regulators[] = {
 	{
 		.desc = {
@@ -368,6 +497,7 @@ static const struct bd718xx_regulator_data bd71847_regulators[] = {
 			.enable_reg = BD718XX_REG_BUCK1_CTRL,
 			.enable_mask = BD718XX_BUCK_EN,
 			.owner = THIS_MODULE,
+			.of_parse_cb = buck1_set_hw_dvs_levels,
 		},
 		.init = {
 			.reg = BD718XX_REG_BUCK1_CTRL,
@@ -391,6 +521,7 @@ static const struct bd718xx_regulator_data bd71847_regulators[] = {
 			.enable_reg = BD718XX_REG_BUCK2_CTRL,
 			.enable_mask = BD718XX_BUCK_EN,
 			.owner = THIS_MODULE,
+			.of_parse_cb = buck2_set_hw_dvs_levels,
 		},
 		.init = {
 			.reg = BD718XX_REG_BUCK2_CTRL,
@@ -662,6 +793,7 @@ static const struct bd718xx_regulator_data bd71837_regulators[] = {
 			.enable_reg = BD718XX_REG_BUCK1_CTRL,
 			.enable_mask = BD718XX_BUCK_EN,
 			.owner = THIS_MODULE,
+			.of_parse_cb = buck1_set_hw_dvs_levels,
 		},
 		.init = {
 			.reg = BD718XX_REG_BUCK1_CTRL,
@@ -685,6 +817,7 @@ static const struct bd718xx_regulator_data bd71837_regulators[] = {
 			.enable_reg = BD718XX_REG_BUCK2_CTRL,
 			.enable_mask = BD718XX_BUCK_EN,
 			.owner = THIS_MODULE,
+			.of_parse_cb = buck2_set_hw_dvs_levels,
 		},
 		.init = {
 			.reg = BD718XX_REG_BUCK2_CTRL,
@@ -708,6 +841,7 @@ static const struct bd718xx_regulator_data bd71837_regulators[] = {
 			.enable_reg = BD71837_REG_BUCK3_CTRL,
 			.enable_mask = BD718XX_BUCK_EN,
 			.owner = THIS_MODULE,
+			.of_parse_cb = buck3_set_hw_dvs_levels,
 		},
 		.init = {
 			.reg = BD71837_REG_BUCK3_CTRL,
@@ -731,6 +865,7 @@ static const struct bd718xx_regulator_data bd71837_regulators[] = {
 			.enable_reg = BD71837_REG_BUCK4_CTRL,
 			.enable_mask = BD718XX_BUCK_EN,
 			.owner = THIS_MODULE,
+			.of_parse_cb = buck4_set_hw_dvs_levels,
 		},
 		.init = {
 			.reg = BD71837_REG_BUCK4_CTRL,
@@ -1029,6 +1164,7 @@ static int bd718xx_probe(struct platform_device *pdev)
 	};
 
 	int i, j, err;
+	bool use_snvs;
 
 	mfd = dev_get_drvdata(pdev->dev.parent);
 	if (!mfd) {
@@ -1055,27 +1191,28 @@ static int bd718xx_probe(struct platform_device *pdev)
 			BD718XX_REG_REGLOCK);
 	}
 
-	/* At poweroff transition PMIC HW disables EN bit for regulators but
-	 * leaves SEL bit untouched. So if state transition from POWEROFF
-	 * is done to SNVS - then all power rails controlled by SW (having
-	 * SEL bit set) stay disabled as EN is cleared. This may result boot
-	 * failure if any crucial systems are powered by these rails.
-	 *
+	use_snvs = of_property_read_bool(pdev->dev.parent->of_node,
+					 "rohm,reset-snvs-powered");
+
+	/*
 	 * Change the next stage from poweroff to be READY instead of SNVS
 	 * for all reset types because OTP loading at READY will clear SEL
 	 * bit allowing HW defaults for power rails to be used
 	 */
-	err = regmap_update_bits(mfd->regmap, BD718XX_REG_TRANS_COND1,
-				 BD718XX_ON_REQ_POWEROFF_MASK |
-				 BD718XX_SWRESET_POWEROFF_MASK |
-				 BD718XX_WDOG_POWEROFF_MASK |
-				 BD718XX_KEY_L_POWEROFF_MASK,
-				 BD718XX_POWOFF_TO_RDY);
-	if (err) {
-		dev_err(&pdev->dev, "Failed to change reset target\n");
-		goto err;
-	} else {
-		dev_dbg(&pdev->dev, "Changed all resets from SVNS to READY\n");
+	if (!use_snvs) {
+		err = regmap_update_bits(mfd->regmap, BD718XX_REG_TRANS_COND1,
+					 BD718XX_ON_REQ_POWEROFF_MASK |
+					 BD718XX_SWRESET_POWEROFF_MASK |
+					 BD718XX_WDOG_POWEROFF_MASK |
+					 BD718XX_KEY_L_POWEROFF_MASK,
+					 BD718XX_POWOFF_TO_RDY);
+		if (err) {
+			dev_err(&pdev->dev, "Failed to change reset target\n");
+			goto err;
+		} else {
+			dev_dbg(&pdev->dev,
+				"Changed all resets from SVNS to READY\n");
+		}
 	}
 
 	for (i = 0; i < pmic_regulators[mfd->chip_type].r_amount; i++) {
@@ -1098,19 +1235,33 @@ static int bd718xx_probe(struct platform_device *pdev)
 			err = PTR_ERR(rdev);
 			goto err;
 		}
-		/* Regulator register gets the regulator constraints and
+
+		/*
+		 * Regulator register gets the regulator constraints and
 		 * applies them (set_machine_constraints). This should have
 		 * turned the control register(s) to correct values and we
 		 * can now switch the control from PMIC state machine to the
 		 * register interface
+		 *
+		 * At poweroff transition PMIC HW disables EN bit for
+		 * regulators but leaves SEL bit untouched. So if state
+		 * transition from POWEROFF is done to SNVS - then all power
+		 * rails controlled by SW (having SEL bit set) stay disabled
+		 * as EN is cleared. This will result boot failure if any
+		 * crucial systems are powered by these rails. We don't
+		 * enable SW control for crucial regulators if snvs state is
+		 * used
 		 */
-		err = regmap_update_bits(mfd->regmap, r->init.reg,
-					 r->init.mask, r->init.val);
-		if (err) {
-			dev_err(&pdev->dev,
-				"Failed to write BUCK/LDO SEL bit for (%s)\n",
-				desc->name);
-			goto err;
+		if (!use_snvs || !rdev->constraints->always_on ||
+		    !rdev->constraints->boot_on) {
+			err = regmap_update_bits(mfd->regmap, r->init.reg,
+						 r->init.mask, r->init.val);
+			if (err) {
+				dev_err(&pdev->dev,
+					"Failed to take control for (%s)\n",
+					desc->name);
+				goto err;
+			}
 		}
 		for (j = 0; j < r->additional_init_amnt; j++) {
 			err = regmap_update_bits(mfd->regmap,
-- 
2.14.3


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes.  Just then, he vanished ~~~

Re: [PATCH v3 1/3] devicetree: bindings: bd718x7: document HW state related ROHM specific properties

[Angus Ainslie]

On 2019-02-14 01:34, Matti Vaittinen wrote:
> Add ROHM BD71837 / BD71847 specific device tree bindings for
> controlling the PMIC shutdown/reset states and voltages for
> different HW states. The PMIC was designed to be used with NXP
> i.MX8 SoC and it supports SNVS low power state which seems to
> be typical for NXP i.MX SoCs. However, when SNVS is used we must
> not allow SW to control enabling/disabling those regulators which
> are crucial for system to boot as there is a HW limitation which
> causes SW controlled regulators to be kept shut down after SNVS
> reset.
> 
> Allow setting the SNVS to be used as reset target state and allow
> marking those regulators which are critical for boot.
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>

One small nit below

Tested-by: Angus Ainslie <angus@akkea.ca>
Reviewed-by: Angus Ainslie <angus@akkea.ca>

> ---
>  .../devicetree/bindings/mfd/rohm,bd71837-pmic.txt  | 17 ++++++++++
>  .../bindings/regulator/rohm,bd71837-regulator.txt  | 38 
> ++++++++++++++++++++++
>  2 files changed, 55 insertions(+)
> 
> diff --git
> a/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
> b/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
> index a4b056761eaa..d5f68ac78d15 100644
> --- a/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
> +++ b/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
> @@ -23,6 +23,20 @@ Required properties:
> 
>  Optional properties:
>  - clock-output-names	: Should contain name for output clock.
> +- rohm,reset-snvs-powered : Transfer BD718x7 to SNVS state at reset.
> +
> +The BD718x7 supports two different HW states as reset target states. 
> States
> +are called as SNVS and READY. At READY state all the PMIC power 
> outputs go
> +down and OTP is reload. At the SNVS state all other logic and external
> +devices apart from the SNVS power domain are shut off. Please refer to 
> NXP
> +i.MX8 documentation for further information regarding SNVS state. When 
> a
> +reset is done via SNVS state the PMIC OTP data is not reload. This 
> causes

Typo s/reload/reloaded/

> +power outputs that have been under SW control to stay down when reset 
> has
> +switched power state to SNVS. If reset is done via READY state the 
> power
> +outputs will be returned to HW control by OTP loading. Thus the reset
> +target state is set to READY by default. If SNVS state is used the 
> boot
> +crucial regulators must have the regulator-always-on and 
> regulator-boot-on
> +properties set in regulator node.
> 
>  Example:
> 
> @@ -43,6 +57,7 @@ Example:
>  		#clock-cells = <0>;
>  		clocks = <&osc 0>;
>  		clock-output-names = "bd71837-32k-out";
> +		rohm,reset-snvs-powered;
> 
>  		regulators {
>  			buck1: BUCK1 {
> @@ -50,8 +65,10 @@ Example:
>  				regulator-min-microvolt = <700000>;
>  				regulator-max-microvolt = <1300000>;
>  				regulator-boot-on;
> +				regulator-always-on;
>  				regulator-ramp-delay = <1250>;
>  			};
> +			// [...]
>  		};
>  	};
> 
> diff --git
> a/Documentation/devicetree/bindings/regulator/rohm,bd71837-regulator.txt
> b/Documentation/devicetree/bindings/regulator/rohm,bd71837-regulator.txt
> index 4b98ca26e61a..cbce62c22b60 100644
> --- 
> a/Documentation/devicetree/bindings/regulator/rohm,bd71837-regulator.txt
> +++ 
> b/Documentation/devicetree/bindings/regulator/rohm,bd71837-regulator.txt
> @@ -27,8 +27,38 @@ BUCK1, BUCK2, BUCK3, BUCK4, BUCK5, BUCK6
>  LDO1, LDO2, LDO3, LDO4, LDO5, LDO6
> 
>  Optional properties:
> +- rohm,dvs-run-voltage		: PMIC default "RUN" state voltage in uV.
> +				  See below table for bucks which support this.
> +- rohm,dvs-idle-voltage		: PMIC default "IDLE" state voltage in uV.
> +				  See below table for bucks which support this.
> +- rohm,dvs-suspend-voltage	: PMIC default "SUSPEND" state voltage in 
> uV.
> +				  See below table for bucks which support this.
>  - Any optional property defined in bindings/regulator/regulator.txt
> 
> +Supported default DVS states:
> +
> +BD71837:
> +buck	| dvs-run-voltage	| dvs-idle-voltage	| dvs-suspend-voltage
> +-----------------------------------------------------------------------------
> +1	| supported		| supported		| supported
> +----------------------------------------------------------------------------
> +2	| supported		| supported		| not supported
> +----------------------------------------------------------------------------
> +3	| supported		| not supported		| not supported
> +----------------------------------------------------------------------------
> +4	| supported		| not supported		| not supported
> +----------------------------------------------------------------------------
> +rest	| not supported		| not supported		| not supported
> +
> +BD71847:
> +buck	| dvs-run-voltage	| dvs-idle-voltage	| dvs-suspend-voltage
> +-----------------------------------------------------------------------------
> +1	| supported		| supported		| supported
> +----------------------------------------------------------------------------
> +2	| supported		| supported		| not supported
> +----------------------------------------------------------------------------
> +rest	| not supported		| not supported		| not supported
> +
>  Example:
>  regulators {
>  	buck1: BUCK1 {
> @@ -36,7 +66,11 @@ regulators {
>  		regulator-min-microvolt = <700000>;
>  		regulator-max-microvolt = <1300000>;
>  		regulator-boot-on;
> +		regulator-always-on;
>  		regulator-ramp-delay = <1250>;
> +		rohm,dvs-run-voltage = <900000>;
> +		rohm,dvs-idle-voltage = <850000>;
> +		rohm,dvs-suspend-voltage = <800000>;
>  	};
>  	buck2: BUCK2 {
>  		regulator-name = "buck2";
> @@ -45,18 +79,22 @@ regulators {
>  		regulator-boot-on;
>  		regulator-always-on;
>  		regulator-ramp-delay = <1250>;
> +		rohm,dvs-run-voltage = <1000000>;
> +		rohm,dvs-idle-voltage = <900000>;
>  	};
>  	buck3: BUCK3 {
>  		regulator-name = "buck3";
>  		regulator-min-microvolt = <700000>;
>  		regulator-max-microvolt = <1300000>;
>  		regulator-boot-on;
> +		rohm,dvs-run-voltage = <1000000>;
>  	};
>  	buck4: BUCK4 {
>  		regulator-name = "buck4";
>  		regulator-min-microvolt = <700000>;
>  		regulator-max-microvolt = <1300000>;
>  		regulator-boot-on;
> +		rohm,dvs-run-voltage = <1000000>;
>  	};
>  	buck5: BUCK5 {
>  		regulator-name = "buck5";
> --
> 2.14.3

Re: [PATCH v3 2/3] regulator: add regulator_desc_list_voltage_linear_range

[Angus Ainslie]

On 2019-02-14 01:38, Matti Vaittinen wrote:
> Add regulator_desc_list_voltage_linear_range which can be used
> by drivers for getting the voltages before regulator is registered.
> This may be useful for drivers which need to fetch the voltage
> selectors at device-tree parsing callback.
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> Acked-by: Mark Brown <broonie@kernel.org>

Tested-by: Angus Ainslie <angus@akkea.ca>
Reviewed-by: Angus Ainslie <angus@akkea.ca>

> ---
> 
> Mark, I converted your:
> "This seems fine." from RFC patch v1 into acked-by. This should
> be unchanged. Please let me know if that's not Ok.
> 
>  drivers/regulator/helpers.c      | 39 
> +++++++++++++++++++++++++++++----------
>  include/linux/regulator/driver.h |  6 ++++++
>  2 files changed, 35 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/regulator/helpers.c b/drivers/regulator/helpers.c
> index 5686a1335bd3..68ac6017ef28 100644
> --- a/drivers/regulator/helpers.c
> +++ b/drivers/regulator/helpers.c
> @@ -594,28 +594,30 @@ int
> regulator_list_voltage_pickable_linear_range(struct regulator_dev
> *rdev,
>  EXPORT_SYMBOL_GPL(regulator_list_voltage_pickable_linear_range);
> 
>  /**
> - * regulator_list_voltage_linear_range - List voltages for linear 
> ranges
> + * regulator_desc_list_voltage_linear_range - List voltages for linear 
> ranges
>   *
> - * @rdev: Regulator device
> + * @desc: Regulator desc for regulator which volatges are to be listed
>   * @selector: Selector to convert into a voltage
>   *
>   * Regulators with a series of simple linear mappings between voltages
> - * and selectors can set linear_ranges in the regulator descriptor and
> - * then use this function as their list_voltage() operation,
> + * and selectors who have set linear_ranges in the regulator 
> descriptor
> + * can use this function prior regulator registration to list 
> voltages.
> + * This is useful when voltages need to be listed during device-tree
> + * parsing.
>   */
> -int regulator_list_voltage_linear_range(struct regulator_dev *rdev,
> -					unsigned int selector)
> +int regulator_desc_list_voltage_linear_range(const struct 
> regulator_desc *desc,
> +					     unsigned int selector)
>  {
>  	const struct regulator_linear_range *range;
>  	int i;
> 
> -	if (!rdev->desc->n_linear_ranges) {
> -		BUG_ON(!rdev->desc->n_linear_ranges);
> +	if (!desc->n_linear_ranges) {
> +		BUG_ON(!desc->n_linear_ranges);
>  		return -EINVAL;
>  	}
> 
> -	for (i = 0; i < rdev->desc->n_linear_ranges; i++) {
> -		range = &rdev->desc->linear_ranges[i];
> +	for (i = 0; i < desc->n_linear_ranges; i++) {
> +		range = &desc->linear_ranges[i];
> 
>  		if (!(selector >= range->min_sel &&
>  		      selector <= range->max_sel))
> @@ -628,6 +630,23 @@ int regulator_list_voltage_linear_range(struct
> regulator_dev *rdev,
> 
>  	return -EINVAL;
>  }
> +EXPORT_SYMBOL_GPL(regulator_desc_list_voltage_linear_range);
> +
> +/**
> + * regulator_list_voltage_linear_range - List voltages for linear 
> ranges
> + *
> + * @rdev: Regulator device
> + * @selector: Selector to convert into a voltage
> + *
> + * Regulators with a series of simple linear mappings between voltages
> + * and selectors can set linear_ranges in the regulator descriptor and
> + * then use this function as their list_voltage() operation,
> + */
> +int regulator_list_voltage_linear_range(struct regulator_dev *rdev,
> +					unsigned int selector)
> +{
> +	return regulator_desc_list_voltage_linear_range(rdev->desc, 
> selector);
> +}
>  EXPORT_SYMBOL_GPL(regulator_list_voltage_linear_range);
> 
>  /**
> diff --git a/include/linux/regulator/driver.h 
> b/include/linux/regulator/driver.h
> index 7f8345bff4e1..05efe2b057c1 100644
> --- a/include/linux/regulator/driver.h
> +++ b/include/linux/regulator/driver.h
> @@ -539,4 +539,10 @@ void *regulator_get_init_drvdata(struct
> regulator_init_data *reg_init_data);
>  void regulator_lock(struct regulator_dev *rdev);
>  void regulator_unlock(struct regulator_dev *rdev);
> 
> +/*
> + * Helper functions intended to be used by regulator drivers prior 
> registering
> + * their regulators.
> + */
> +int regulator_desc_list_voltage_linear_range(const struct 
> regulator_desc *desc,
> +					     unsigned int selector);
>  #endif
> --
> 2.14.3

Re: [PATCH v3 3/3] regulator: bd718x7: Support SNVS low power state

[Angus Ainslie]

On 2019-02-14 01:39, Matti Vaittinen wrote:
> read ROHM BD71837 / BD71847 specific device tree bindings for
> controlling the PMIC shutdown/reset states and voltages for
> different HW states. The PMIC was designed to be used with NXP
> i.MX8 SoC and it supports SNVS low power state which seems to
> be typical for NXP i.MX SoCs. However, when SNVS is used we must
> not allow SW to control enabling/disabling those regulators which
> are crucial for system to boot as there is a HW limitation which
> causes SW controlled regulators to be kept shut down after SNVS
> reset.
> 
> Allow setting the SNVS to be used as reset target state and allow
> marking those regulators which are critical for boot.
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>

Tested-by: Angus Ainslie <angus@akkea.ca>
Reviewed-by: Angus Ainslie <angus@akkea.ca>

> ---
> 
> Angus, I would be grateful if you have a chance to test this
> on your system :)
> 
>  drivers/regulator/bd718x7-regulator.c | 201 
> +++++++++++++++++++++++++++++-----
>  1 file changed, 176 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/regulator/bd718x7-regulator.c
> b/drivers/regulator/bd718x7-regulator.c
> index ccea133778c8..b2191be49670 100644
> --- a/drivers/regulator/bd718x7-regulator.c
> +++ b/drivers/regulator/bd718x7-regulator.c
> @@ -350,6 +350,135 @@ static const struct reg_init bd71837_ldo6_inits[] 
> = {
>  	},
>  };
> 
> +#define NUM_DVS_BUCKS 4
> +
> +struct of_dvs_setting {
> +	const char *prop;
> +	unsigned int reg;
> +};
> +
> +static int set_dvs_levels(const struct of_dvs_setting *dvs,
> +			  struct device_node *np,
> +			  const struct regulator_desc *desc,
> +			  struct regmap *regmap)
> +{
> +	int ret, i;
> +	unsigned int uv;
> +
> +	ret = of_property_read_u32(np, dvs->prop, &uv);
> +	if (ret) {
> +		if (ret != -EINVAL)
> +			return ret;
> +		return 0;
> +	}
> +
> +	for (i = 0; i < desc->n_voltages; i++) {
> +		ret = regulator_desc_list_voltage_linear_range(desc, i);
> +		if (ret < 0)
> +			continue;
> +		if (ret == uv) {
> +			i <<= ffs(desc->vsel_mask) - 1;
> +			ret = regmap_update_bits(regmap, dvs->reg,
> +						 DVS_BUCK_RUN_MASK, i);
> +			break;
> +		}
> +	}
> +	return ret;
> +}
> +
> +static int buck4_set_hw_dvs_levels(struct device_node *np,
> +			    const struct regulator_desc *desc,
> +			    struct regulator_config *cfg)
> +{
> +	int ret, i;
> +	const struct of_dvs_setting dvs[] = {
> +		{
> +			.prop = "rohm,dvs-run-voltage",
> +			.reg = BD71837_REG_BUCK4_VOLT_RUN,
> +		},
> +	};
> +
> +	for (i = 0; i < ARRAY_SIZE(dvs); i++) {
> +		ret = set_dvs_levels(&dvs[i], np, desc, cfg->regmap);
> +		if (ret)
> +			break;
> +	}
> +	return ret;
> +}
> +static int buck3_set_hw_dvs_levels(struct device_node *np,
> +			    const struct regulator_desc *desc,
> +			    struct regulator_config *cfg)
> +{
> +	int ret, i;
> +	const struct of_dvs_setting dvs[] = {
> +		{
> +			.prop = "rohm,dvs-run-voltage",
> +			.reg = BD71837_REG_BUCK3_VOLT_RUN,
> +		},
> +	};
> +
> +	for (i = 0; i < ARRAY_SIZE(dvs); i++) {
> +		ret = set_dvs_levels(&dvs[i], np, desc, cfg->regmap);
> +		if (ret)
> +			break;
> +	}
> +	return ret;
> +}
> +
> +static int buck2_set_hw_dvs_levels(struct device_node *np,
> +			    const struct regulator_desc *desc,
> +			    struct regulator_config *cfg)
> +{
> +	int ret, i;
> +	const struct of_dvs_setting dvs[] = {
> +		{
> +			.prop = "rohm,dvs-run-voltage",
> +			.reg = BD718XX_REG_BUCK2_VOLT_RUN,
> +		},
> +		{
> +			.prop = "rohm,dvs-idle-voltage",
> +			.reg = BD718XX_REG_BUCK2_VOLT_IDLE,
> +		},
> +	};
> +
> +
> +
> +	for (i = 0; i < ARRAY_SIZE(dvs); i++) {
> +		ret = set_dvs_levels(&dvs[i], np, desc, cfg->regmap);
> +		if (ret)
> +			break;
> +	}
> +	return ret;
> +}
> +
> +static int buck1_set_hw_dvs_levels(struct device_node *np,
> +			    const struct regulator_desc *desc,
> +			    struct regulator_config *cfg)
> +{
> +	int ret, i;
> +	const struct of_dvs_setting dvs[] = {
> +		{
> +			.prop = "rohm,dvs-run-voltage",
> +			.reg = BD718XX_REG_BUCK1_VOLT_RUN,
> +		},
> +		{
> +			.prop = "rohm,dvs-idle-voltage",
> +			.reg = BD718XX_REG_BUCK1_VOLT_IDLE,
> +		},
> +		{
> +			.prop = "rohm,dvs-suspend-voltage",
> +			.reg = BD718XX_REG_BUCK1_VOLT_SUSP,
> +		},
> +	};
> +
> +	for (i = 0; i < ARRAY_SIZE(dvs); i++) {
> +		ret = set_dvs_levels(&dvs[i], np, desc, cfg->regmap);
> +		if (ret)
> +			break;
> +	}
> +	return ret;
> +}
> +
>  static const struct bd718xx_regulator_data bd71847_regulators[] = {
>  	{
>  		.desc = {
> @@ -368,6 +497,7 @@ static const struct bd718xx_regulator_data
> bd71847_regulators[] = {
>  			.enable_reg = BD718XX_REG_BUCK1_CTRL,
>  			.enable_mask = BD718XX_BUCK_EN,
>  			.owner = THIS_MODULE,
> +			.of_parse_cb = buck1_set_hw_dvs_levels,
>  		},
>  		.init = {
>  			.reg = BD718XX_REG_BUCK1_CTRL,
> @@ -391,6 +521,7 @@ static const struct bd718xx_regulator_data
> bd71847_regulators[] = {
>  			.enable_reg = BD718XX_REG_BUCK2_CTRL,
>  			.enable_mask = BD718XX_BUCK_EN,
>  			.owner = THIS_MODULE,
> +			.of_parse_cb = buck2_set_hw_dvs_levels,
>  		},
>  		.init = {
>  			.reg = BD718XX_REG_BUCK2_CTRL,
> @@ -662,6 +793,7 @@ static const struct bd718xx_regulator_data
> bd71837_regulators[] = {
>  			.enable_reg = BD718XX_REG_BUCK1_CTRL,
>  			.enable_mask = BD718XX_BUCK_EN,
>  			.owner = THIS_MODULE,
> +			.of_parse_cb = buck1_set_hw_dvs_levels,
>  		},
>  		.init = {
>  			.reg = BD718XX_REG_BUCK1_CTRL,
> @@ -685,6 +817,7 @@ static const struct bd718xx_regulator_data
> bd71837_regulators[] = {
>  			.enable_reg = BD718XX_REG_BUCK2_CTRL,
>  			.enable_mask = BD718XX_BUCK_EN,
>  			.owner = THIS_MODULE,
> +			.of_parse_cb = buck2_set_hw_dvs_levels,
>  		},
>  		.init = {
>  			.reg = BD718XX_REG_BUCK2_CTRL,
> @@ -708,6 +841,7 @@ static const struct bd718xx_regulator_data
> bd71837_regulators[] = {
>  			.enable_reg = BD71837_REG_BUCK3_CTRL,
>  			.enable_mask = BD718XX_BUCK_EN,
>  			.owner = THIS_MODULE,
> +			.of_parse_cb = buck3_set_hw_dvs_levels,
>  		},
>  		.init = {
>  			.reg = BD71837_REG_BUCK3_CTRL,
> @@ -731,6 +865,7 @@ static const struct bd718xx_regulator_data
> bd71837_regulators[] = {
>  			.enable_reg = BD71837_REG_BUCK4_CTRL,
>  			.enable_mask = BD718XX_BUCK_EN,
>  			.owner = THIS_MODULE,
> +			.of_parse_cb = buck4_set_hw_dvs_levels,
>  		},
>  		.init = {
>  			.reg = BD71837_REG_BUCK4_CTRL,
> @@ -1029,6 +1164,7 @@ static int bd718xx_probe(struct platform_device 
> *pdev)
>  	};
> 
>  	int i, j, err;
> +	bool use_snvs;
> 
>  	mfd = dev_get_drvdata(pdev->dev.parent);
>  	if (!mfd) {
> @@ -1055,27 +1191,28 @@ static int bd718xx_probe(struct platform_device 
> *pdev)
>  			BD718XX_REG_REGLOCK);
>  	}
> 
> -	/* At poweroff transition PMIC HW disables EN bit for regulators but
> -	 * leaves SEL bit untouched. So if state transition from POWEROFF
> -	 * is done to SNVS - then all power rails controlled by SW (having
> -	 * SEL bit set) stay disabled as EN is cleared. This may result boot
> -	 * failure if any crucial systems are powered by these rails.
> -	 *
> +	use_snvs = of_property_read_bool(pdev->dev.parent->of_node,
> +					 "rohm,reset-snvs-powered");
> +
> +	/*
>  	 * Change the next stage from poweroff to be READY instead of SNVS
>  	 * for all reset types because OTP loading at READY will clear SEL
>  	 * bit allowing HW defaults for power rails to be used
>  	 */
> -	err = regmap_update_bits(mfd->regmap, BD718XX_REG_TRANS_COND1,
> -				 BD718XX_ON_REQ_POWEROFF_MASK |
> -				 BD718XX_SWRESET_POWEROFF_MASK |
> -				 BD718XX_WDOG_POWEROFF_MASK |
> -				 BD718XX_KEY_L_POWEROFF_MASK,
> -				 BD718XX_POWOFF_TO_RDY);
> -	if (err) {
> -		dev_err(&pdev->dev, "Failed to change reset target\n");
> -		goto err;
> -	} else {
> -		dev_dbg(&pdev->dev, "Changed all resets from SVNS to READY\n");
> +	if (!use_snvs) {
> +		err = regmap_update_bits(mfd->regmap, BD718XX_REG_TRANS_COND1,
> +					 BD718XX_ON_REQ_POWEROFF_MASK |
> +					 BD718XX_SWRESET_POWEROFF_MASK |
> +					 BD718XX_WDOG_POWEROFF_MASK |
> +					 BD718XX_KEY_L_POWEROFF_MASK,
> +					 BD718XX_POWOFF_TO_RDY);
> +		if (err) {
> +			dev_err(&pdev->dev, "Failed to change reset target\n");
> +			goto err;
> +		} else {
> +			dev_dbg(&pdev->dev,
> +				"Changed all resets from SVNS to READY\n");
> +		}
>  	}
> 
>  	for (i = 0; i < pmic_regulators[mfd->chip_type].r_amount; i++) {
> @@ -1098,19 +1235,33 @@ static int bd718xx_probe(struct platform_device 
> *pdev)
>  			err = PTR_ERR(rdev);
>  			goto err;
>  		}
> -		/* Regulator register gets the regulator constraints and
> +
> +		/*
> +		 * Regulator register gets the regulator constraints and
>  		 * applies them (set_machine_constraints). This should have
>  		 * turned the control register(s) to correct values and we
>  		 * can now switch the control from PMIC state machine to the
>  		 * register interface
> +		 *
> +		 * At poweroff transition PMIC HW disables EN bit for
> +		 * regulators but leaves SEL bit untouched. So if state
> +		 * transition from POWEROFF is done to SNVS - then all power
> +		 * rails controlled by SW (having SEL bit set) stay disabled
> +		 * as EN is cleared. This will result boot failure if any
> +		 * crucial systems are powered by these rails. We don't
> +		 * enable SW control for crucial regulators if snvs state is
> +		 * used
>  		 */
> -		err = regmap_update_bits(mfd->regmap, r->init.reg,
> -					 r->init.mask, r->init.val);
> -		if (err) {
> -			dev_err(&pdev->dev,
> -				"Failed to write BUCK/LDO SEL bit for (%s)\n",
> -				desc->name);
> -			goto err;
> +		if (!use_snvs || !rdev->constraints->always_on ||
> +		    !rdev->constraints->boot_on) {
> +			err = regmap_update_bits(mfd->regmap, r->init.reg,
> +						 r->init.mask, r->init.val);
> +			if (err) {
> +				dev_err(&pdev->dev,
> +					"Failed to take control for (%s)\n",
> +					desc->name);
> +				goto err;
> +			}
>  		}
>  		for (j = 0; j < r->additional_init_amnt; j++) {
>  			err = regmap_update_bits(mfd->regmap,
> --
> 2.14.3

Re: [PATCH v3 1/3] devicetree: bindings: bd718x7: document HW state related ROHM specific properties

[Lee Jones]

On Thu, 14 Feb 2019, Matti Vaittinen wrote:

> Add ROHM BD71837 / BD71847 specific device tree bindings for
> controlling the PMIC shutdown/reset states and voltages for
> different HW states. The PMIC was designed to be used with NXP
> i.MX8 SoC and it supports SNVS low power state which seems to
> be typical for NXP i.MX SoCs. However, when SNVS is used we must
> not allow SW to control enabling/disabling those regulators which
> are crucial for system to boot as there is a HW limitation which
> causes SW controlled regulators to be kept shut down after SNVS
> reset.
> 
> Allow setting the SNVS to be used as reset target state and allow
> marking those regulators which are critical for boot.
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> ---
>  .../devicetree/bindings/mfd/rohm,bd71837-pmic.txt  | 17 ++++++++++
>  .../bindings/regulator/rohm,bd71837-regulator.txt  | 38 ++++++++++++++++++++++
>  2 files changed, 55 insertions(+)

Needs a DT Ack.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 1/3] devicetree: bindings: bd718x7: document HW state related ROHM specific properties

[Matti Vaittinen]

Hello Lee, All

On Wed, Mar 20, 2019 at 01:02:59PM +0000, Lee Jones wrote:
> On Thu, 14 Feb 2019, Matti Vaittinen wrote:
> 
> > Add ROHM BD71837 / BD71847 specific device tree bindings for
> > controlling the PMIC shutdown/reset states and voltages for
> > different HW states. The PMIC was designed to be used with NXP
> > i.MX8 SoC and it supports SNVS low power state which seems to
> > be typical for NXP i.MX SoCs. However, when SNVS is used we must
> > not allow SW to control enabling/disabling those regulators which
> > are crucial for system to boot as there is a HW limitation which
> > causes SW controlled regulators to be kept shut down after SNVS
> > reset.
> > 
> > Allow setting the SNVS to be used as reset target state and allow
> > marking those regulators which are critical for boot.
> > 
> > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> > ---
> >  .../devicetree/bindings/mfd/rohm,bd71837-pmic.txt  | 17 ++++++++++
> >  .../bindings/regulator/rohm,bd71837-regulator.txt  | 38 ++++++++++++++++++++++
> >  2 files changed, 55 insertions(+)
> 
> Needs a DT Ack.

I think this was already applied to regulator tree by Mark:
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git/commit/?h=for-next&id=f43d1b388f9be4aa47ed42c33659243a675c5c76
Does it go in via his tree or do we need to apply it in Lee's tree too?

Br,
	Matti Vaittinen

> 
> -- 
> Lee Jones [李琼斯]
> Linaro Services Technical Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~

Re: [PATCH v3 1/3] devicetree: bindings: bd718x7: document HW state related ROHM specific properties

[Lee Jones]

On Wed, 20 Mar 2019, Matti Vaittinen wrote:

> Hello Lee, All
> 
> On Wed, Mar 20, 2019 at 01:02:59PM +0000, Lee Jones wrote:
> > On Thu, 14 Feb 2019, Matti Vaittinen wrote:
> > 
> > > Add ROHM BD71837 / BD71847 specific device tree bindings for
> > > controlling the PMIC shutdown/reset states and voltages for
> > > different HW states. The PMIC was designed to be used with NXP
> > > i.MX8 SoC and it supports SNVS low power state which seems to
> > > be typical for NXP i.MX SoCs. However, when SNVS is used we must
> > > not allow SW to control enabling/disabling those regulators which
> > > are crucial for system to boot as there is a HW limitation which
> > > causes SW controlled regulators to be kept shut down after SNVS
> > > reset.
> > > 
> > > Allow setting the SNVS to be used as reset target state and allow
> > > marking those regulators which are critical for boot.
> > > 
> > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> > > ---
> > >  .../devicetree/bindings/mfd/rohm,bd71837-pmic.txt  | 17 ++++++++++
> > >  .../bindings/regulator/rohm,bd71837-regulator.txt  | 38 ++++++++++++++++++++++
> > >  2 files changed, 55 insertions(+)
> > 
> > Needs a DT Ack.
> 
> I think this was already applied to regulator tree by Mark:
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git/commit/?h=for-next&id=f43d1b388f9be4aa47ed42c33659243a675c5c76
> Does it go in via his tree or do we need to apply it in Lee's tree too?

Why don't I see an email from Mark?

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 1/3] devicetree: bindings: bd718x7: document HW state related ROHM specific properties

[Vaittinen, Matti]

On Thu, 2019-03-21 at 08:23 +0000, Lee Jones wrote:
> On Wed, 20 Mar 2019, Matti Vaittinen wrote:
> 
> > Hello Lee, All
> > 
> > On Wed, Mar 20, 2019 at 01:02:59PM +0000, Lee Jones wrote:
> > > On Thu, 14 Feb 2019, Matti Vaittinen wrote:
> > > 
> > > > Add ROHM BD71837 / BD71847 specific device tree bindings for
> > > > controlling the PMIC shutdown/reset states and voltages for
> > > > different HW states. The PMIC was designed to be used with NXP
> > > > i.MX8 SoC and it supports SNVS low power state which seems to
> > > > be typical for NXP i.MX SoCs. However, when SNVS is used we
> > > > must
> > > > not allow SW to control enabling/disabling those regulators
> > > > which
> > > > are crucial for system to boot as there is a HW limitation
> > > > which
> > > > causes SW controlled regulators to be kept shut down after SNVS
> > > > reset.
> > > > 
> > > > Allow setting the SNVS to be used as reset target state and
> > > > allow
> > > > marking those regulators which are critical for boot.
> > > > 
> > > > Signed-off-by: Matti Vaittinen <
> > > > matti.vaittinen@fi.rohmeurope.com>
> > > > ---
> > > >  .../devicetree/bindings/mfd/rohm,bd71837-pmic.txt  | 17
> > > > ++++++++++
> > > >  .../bindings/regulator/rohm,bd71837-regulator.txt  | 38
> > > > ++++++++++++++++++++++
> > > >  2 files changed, 55 insertions(+)
> > > 
> > > Needs a DT Ack.
> > 
> > I think this was already applied to regulator tree by Mark:
> > 
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git/commit/?h=for-next&id=f43d1b388f9be4aa47ed42c33659243a675c5c76
> > Does it go in via his tree or do we need to apply it in Lee's tree
> > too?
> 
> Why don't I see an email from Mark?
> 


https://lore.kernel.org/lkml/20190214160025.65B8E1126F78@debutante.sirena.org.uk/

Maybe that's what you're looking for?

Br,
	Matti Vaittinen

Re: [PATCH v3 1/3] devicetree: bindings: bd718x7: document HW state related ROHM specific properties

[Lee Jones]

On Thu, 21 Mar 2019, Vaittinen, Matti wrote:

> On Thu, 2019-03-21 at 08:23 +0000, Lee Jones wrote:
> > On Wed, 20 Mar 2019, Matti Vaittinen wrote:
> > 
> > > Hello Lee, All
> > > 
> > > On Wed, Mar 20, 2019 at 01:02:59PM +0000, Lee Jones wrote:
> > > > On Thu, 14 Feb 2019, Matti Vaittinen wrote:
> > > > 
> > > > > Add ROHM BD71837 / BD71847 specific device tree bindings for
> > > > > controlling the PMIC shutdown/reset states and voltages for
> > > > > different HW states. The PMIC was designed to be used with NXP
> > > > > i.MX8 SoC and it supports SNVS low power state which seems to
> > > > > be typical for NXP i.MX SoCs. However, when SNVS is used we
> > > > > must
> > > > > not allow SW to control enabling/disabling those regulators
> > > > > which
> > > > > are crucial for system to boot as there is a HW limitation
> > > > > which
> > > > > causes SW controlled regulators to be kept shut down after SNVS
> > > > > reset.
> > > > > 
> > > > > Allow setting the SNVS to be used as reset target state and
> > > > > allow
> > > > > marking those regulators which are critical for boot.
> > > > > 
> > > > > Signed-off-by: Matti Vaittinen <
> > > > > matti.vaittinen@fi.rohmeurope.com>
> > > > > ---
> > > > >  .../devicetree/bindings/mfd/rohm,bd71837-pmic.txt  | 17
> > > > > ++++++++++
> > > > >  .../bindings/regulator/rohm,bd71837-regulator.txt  | 38
> > > > > ++++++++++++++++++++++
> > > > >  2 files changed, 55 insertions(+)
> > > > 
> > > > Needs a DT Ack.
> > > 
> > > I think this was already applied to regulator tree by Mark:
> > > 
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git/commit/?h=for-next&id=f43d1b388f9be4aa47ed42c33659243a675c5c76
> > > Does it go in via his tree or do we need to apply it in Lee's tree
> > > too?
> > 
> > Why don't I see an email from Mark?
> 
> https://lore.kernel.org/lkml/20190214160025.65B8E1126F78@debutante.sirena.org.uk/
> 
> Maybe that's what you're looking for?

It is, but I need this in my inbox.

Mark, could you please take the time to fix your scripts please?

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog