lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20230419-dynamic-vmon-v3-5-4179b586d8a1@skidata.com>
Date:   Sun, 21 May 2023 13:39:54 +0200
From:   Benjamin Bara <bbara93@...il.com>
To:     Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>
Cc:     support.opensource@...semi.com,
        DLG-Adam.Ward.opensource@...renesas.com,
        linux-kernel@...r.kernel.org,
        Matti Vaittinen <mazziesaccount@...il.com>,
        Benjamin Bara <benjamin.bara@...data.com>
Subject: [PATCH RFC v3 5/5] regulator: bd718x7: let the core handle the
 monitors

From: Benjamin Bara <benjamin.bara@...data.com>

The monitors of the bd718x7 must be disabled while the respective
regulator is switching to a higher voltage. Use the new property
'.mon_disable_reg_set_higher' to activate the handling in the core.

.mon_disable_reg_set_higher requires get_active_protections() to find
out if a regulator is monitored without the device-tree knowing it.
Otherwise, this might lead to a failure as the core might not be aware
that a regulator is monitored and therefore would not disable it.
Therefore, implement it.

Signed-off-by: Benjamin Bara <benjamin.bara@...data.com>
---
 drivers/regulator/bd718x7-regulator.c | 206 +++++++++++-----------------------
 1 file changed, 66 insertions(+), 140 deletions(-)

diff --git a/drivers/regulator/bd718x7-regulator.c b/drivers/regulator/bd718x7-regulator.c
index b0b9938c20a1..897388d68949 100644
--- a/drivers/regulator/bd718x7-regulator.c
+++ b/drivers/regulator/bd718x7-regulator.c
@@ -56,7 +56,7 @@
 
 #define BD718XX_OPS(name, _list_voltage, _map_voltage, _set_voltage_sel, \
 		   _get_voltage_sel, _set_voltage_time_sel, _set_ramp_delay, \
-		   _set_uvp, _set_ovp)				\
+		   _set_uvp, _set_ovp, _get_prot)		\
 static const struct regulator_ops name = {			\
 	.enable = regulator_enable_regmap,			\
 	.disable = regulator_disable_regmap,			\
@@ -69,6 +69,7 @@ static const struct regulator_ops name = {			\
 	.set_ramp_delay = (_set_ramp_delay),			\
 	.set_under_voltage_protection = (_set_uvp),		\
 	.set_over_voltage_protection = (_set_ovp),		\
+	.get_active_protections = (_get_prot),			\
 };								\
 								\
 static const struct regulator_ops BD718XX_HWOPNAME(name) = {	\
@@ -81,6 +82,7 @@ static const struct regulator_ops BD718XX_HWOPNAME(name) = {	\
 	.set_ramp_delay = (_set_ramp_delay),			\
 	.set_under_voltage_protection = (_set_uvp),		\
 	.set_over_voltage_protection = (_set_ovp),		\
+	.get_active_protections = (_get_prot),			\
 }								\
 
 /*
@@ -126,128 +128,6 @@ static int bd71837_get_buck34_enable_hwctrl(struct regulator_dev *rdev)
 	return !!(BD718XX_BUCK_RUN_ON & val);
 }
 
-static void voltage_change_done(struct regulator_dev *rdev, unsigned int sel,
-				unsigned int *mask)
-{
-	int ret;
-
-	if (*mask) {
-		/*
-		 * Let's allow scheduling as we use I2C anyways. We just need to
-		 * guarantee minimum of 1ms sleep - it shouldn't matter if we
-		 * exceed it due to the scheduling.
-		 */
-		msleep(1);
-
-		ret = regmap_clear_bits(rdev->regmap, BD718XX_REG_MVRFLTMASK2,
-					 *mask);
-		if (ret)
-			dev_err(&rdev->dev,
-				"Failed to re-enable voltage monitoring (%d)\n",
-				ret);
-	}
-}
-
-static int voltage_change_prepare(struct regulator_dev *rdev, unsigned int sel,
-				  unsigned int *mask)
-{
-	int ret;
-
-	*mask = 0;
-	if (rdev->desc->ops->is_enabled(rdev)) {
-		int now, new;
-
-		now = rdev->desc->ops->get_voltage_sel(rdev);
-		if (now < 0)
-			return now;
-
-		now = rdev->desc->ops->list_voltage(rdev, now);
-		if (now < 0)
-			return now;
-
-		new = rdev->desc->ops->list_voltage(rdev, sel);
-		if (new < 0)
-			return new;
-
-		/*
-		 * If we increase LDO voltage when LDO is enabled we need to
-		 * disable the power-good detection until voltage has reached
-		 * the new level. According to HW colleagues the maximum time
-		 * it takes is 1000us. I assume that on systems with light load
-		 * this might be less - and we could probably use DT to give
-		 * system specific delay value if performance matters.
-		 *
-		 * Well, knowing we use I2C here and can add scheduling delays
-		 * I don't think it is worth the hassle and I just add fixed
-		 * 1ms sleep here (and allow scheduling). If this turns out to
-		 * be a problem we can change it to delay and make the delay
-		 * time configurable.
-		 */
-		if (new > now) {
-			int tmp;
-			int prot_bit;
-			int ldo_offset = rdev->desc->id - BD718XX_LDO1;
-
-			prot_bit = BD718XX_LDO1_VRMON80 << ldo_offset;
-			ret = regmap_read(rdev->regmap, BD718XX_REG_MVRFLTMASK2,
-					  &tmp);
-			if (ret) {
-				dev_err(&rdev->dev,
-					"Failed to read voltage monitoring state\n");
-				return ret;
-			}
-
-			if (!(tmp & prot_bit)) {
-				/* We disable protection if it was enabled... */
-				ret = regmap_set_bits(rdev->regmap,
-						      BD718XX_REG_MVRFLTMASK2,
-						      prot_bit);
-				/* ...and we also want to re-enable it */
-				*mask = prot_bit;
-			}
-			if (ret) {
-				dev_err(&rdev->dev,
-					"Failed to stop voltage monitoring\n");
-				return ret;
-			}
-		}
-	}
-
-	return 0;
-}
-
-static int bd718xx_set_voltage_sel_restricted(struct regulator_dev *rdev,
-						    unsigned int sel)
-{
-	int ret;
-	int mask;
-
-	ret = voltage_change_prepare(rdev, sel, &mask);
-	if (ret)
-		return ret;
-
-	ret = regulator_set_voltage_sel_regmap(rdev, sel);
-	voltage_change_done(rdev, sel, &mask);
-
-	return ret;
-}
-
-static int bd718xx_set_voltage_sel_pickable_restricted(
-		struct regulator_dev *rdev, unsigned int sel)
-{
-	int ret;
-	int mask;
-
-	ret = voltage_change_prepare(rdev, sel, &mask);
-	if (ret)
-		return ret;
-
-	ret = regulator_set_voltage_sel_pickable_regmap(rdev, sel);
-	voltage_change_done(rdev, sel, &mask);
-
-	return ret;
-}
-
 static int bd71837_set_voltage_sel_pickable_restricted(
 		struct regulator_dev *rdev, unsigned int sel)
 {
@@ -457,6 +337,21 @@ static int bd718x7_xvp_sanity_check(struct regulator_dev *rdev, int lim_uV,
 	return 0;
 }
 
+static int bd717x7_get_ldo_prot(struct regulator_dev *rdev, unsigned int *val)
+{
+	int ldo_offset = rdev->desc->id - BD718XX_LDO1;
+	int prot_bit = BD718XX_LDO1_VRMON80 << ldo_offset;
+	int ret;
+
+	ret = regmap_test_bits(rdev->regmap, BD718XX_REG_MVRFLTMASK2, prot_bit);
+	if (ret < 0)
+		return ret;
+	if (ret)
+		*val |= REGULATOR_MONITOR_UNDER_VOLTAGE;
+
+	return 0;
+}
+
 static int bd718x7_set_ldo_uvp(struct regulator_dev *rdev, int lim_uV,
 			       int severity, bool enable)
 {
@@ -519,6 +414,31 @@ static int bd718x7_get_buck_uvp_info(int id, int *reg, int *bit)
 	return 0;
 }
 
+static int bd717x7_get_buck_prot(struct regulator_dev *rdev, unsigned int *val)
+{
+	int ret, reg, bit;
+
+	ret = bd718x7_get_buck_uvp_info(rdev->desc->id, &reg, &bit);
+	if (ret)
+		return ret;
+	ret = regmap_test_bits(rdev->regmap, reg, bit);
+	if (ret < 0)
+		return ret;
+	if (ret)
+		*val |= REGULATOR_MONITOR_UNDER_VOLTAGE;
+
+	ret = bd718x7_get_buck_ovp_info(rdev->desc->id, &reg, &bit);
+	if (ret)
+		return ret;
+	ret = regmap_test_bits(rdev->regmap, reg, bit);
+	if (ret < 0)
+		return ret;
+	if (ret)
+		*val |= REGULATOR_MONITOR_OVER_VOLTAGE;
+
+	return 0;
+}
+
 static int bd718x7_set_buck_uvp(struct regulator_dev *rdev, int lim_uV,
 				int severity, bool enable)
 {
@@ -564,15 +484,15 @@ static int bd718x7_set_buck_ovp(struct regulator_dev *rdev, int lim_uV,
  */
 BD718XX_OPS(bd718xx_pickable_range_ldo_ops,
 	    regulator_list_voltage_pickable_linear_range, NULL,
-	    bd718xx_set_voltage_sel_pickable_restricted,
+	    regulator_set_voltage_sel_pickable_regmap,
 	    regulator_get_voltage_sel_pickable_regmap, NULL, NULL,
-	    bd718x7_set_ldo_uvp, NULL);
+	    bd718x7_set_ldo_uvp, NULL, bd717x7_get_ldo_prot);
 
 /* BD71847 and BD71850 LDO 5 is by default OFF at RUN state */
 static const struct regulator_ops bd718xx_ldo5_ops_hwstate = {
 	.is_enabled = never_enabled_by_hwstate,
 	.list_voltage = regulator_list_voltage_pickable_linear_range,
-	.set_voltage_sel = bd718xx_set_voltage_sel_pickable_restricted,
+	.set_voltage_sel = regulator_set_voltage_sel_pickable_regmap,
 	.get_voltage_sel = regulator_get_voltage_sel_pickable_regmap,
 	.set_under_voltage_protection = bd718x7_set_ldo_uvp,
 };
@@ -582,27 +502,27 @@ BD718XX_OPS(bd718xx_pickable_range_buck_ops,
 	    regulator_set_voltage_sel_pickable_regmap,
 	    regulator_get_voltage_sel_pickable_regmap,
 	    regulator_set_voltage_time_sel, NULL, bd718x7_set_buck_uvp,
-	    bd718x7_set_buck_ovp);
+	    bd718x7_set_buck_ovp, bd717x7_get_buck_prot);
 
 BD718XX_OPS(bd718xx_ldo_regulator_ops, regulator_list_voltage_linear_range,
-	    NULL, bd718xx_set_voltage_sel_restricted,
+	    NULL, regulator_set_voltage_sel_regmap,
 	    regulator_get_voltage_sel_regmap, NULL, NULL, bd718x7_set_ldo_uvp,
-	    NULL);
+	    NULL, bd717x7_get_ldo_prot);
 
 BD718XX_OPS(bd718xx_ldo_regulator_nolinear_ops, regulator_list_voltage_table,
-	    NULL, bd718xx_set_voltage_sel_restricted,
+	    NULL, regulator_set_voltage_sel_regmap,
 	    regulator_get_voltage_sel_regmap, NULL, NULL, bd718x7_set_ldo_uvp,
-	    NULL);
+	    NULL, bd717x7_get_ldo_prot);
 
 BD718XX_OPS(bd718xx_buck_regulator_ops, regulator_list_voltage_linear_range,
 	    NULL, regulator_set_voltage_sel_regmap,
 	    regulator_get_voltage_sel_regmap, regulator_set_voltage_time_sel,
-	    NULL, bd718x7_set_buck_uvp, bd718x7_set_buck_ovp);
+	    NULL, bd718x7_set_buck_uvp, bd718x7_set_buck_ovp, bd717x7_get_buck_prot);
 
 BD718XX_OPS(bd718xx_buck_regulator_nolinear_ops, regulator_list_voltage_table,
 	    regulator_map_voltage_ascend, regulator_set_voltage_sel_regmap,
 	    regulator_get_voltage_sel_regmap, regulator_set_voltage_time_sel,
-	    NULL, bd718x7_set_buck_uvp, bd718x7_set_buck_ovp);
+	    NULL, bd718x7_set_buck_uvp, bd718x7_set_buck_ovp, bd717x7_get_buck_prot);
 
 /*
  * OPS for BD71837
@@ -611,34 +531,34 @@ BD718XX_OPS(bd71837_pickable_range_ldo_ops,
 	    regulator_list_voltage_pickable_linear_range, NULL,
 	    bd71837_set_voltage_sel_pickable_restricted,
 	    regulator_get_voltage_sel_pickable_regmap, NULL, NULL,
-	    bd718x7_set_ldo_uvp, NULL);
+	    bd718x7_set_ldo_uvp, NULL, bd717x7_get_ldo_prot);
 
 BD718XX_OPS(bd71837_pickable_range_buck_ops,
 	    regulator_list_voltage_pickable_linear_range, NULL,
 	    bd71837_set_voltage_sel_pickable_restricted,
 	    regulator_get_voltage_sel_pickable_regmap,
 	    regulator_set_voltage_time_sel, NULL, bd718x7_set_buck_uvp,
-	    bd718x7_set_buck_ovp);
+	    bd718x7_set_buck_ovp, bd717x7_get_buck_prot);
 
 BD718XX_OPS(bd71837_ldo_regulator_ops, regulator_list_voltage_linear_range,
 	    NULL, rohm_regulator_set_voltage_sel_restricted,
 	    regulator_get_voltage_sel_regmap, NULL, NULL, bd718x7_set_ldo_uvp,
-	    NULL);
+	    NULL, bd717x7_get_ldo_prot);
 
 BD718XX_OPS(bd71837_ldo_regulator_nolinear_ops, regulator_list_voltage_table,
 	    NULL, rohm_regulator_set_voltage_sel_restricted,
 	    regulator_get_voltage_sel_regmap, NULL, NULL, bd718x7_set_ldo_uvp,
-	    NULL);
+	    NULL, bd717x7_get_ldo_prot);
 
 BD718XX_OPS(bd71837_buck_regulator_ops, regulator_list_voltage_linear_range,
 	    NULL, rohm_regulator_set_voltage_sel_restricted,
 	    regulator_get_voltage_sel_regmap, regulator_set_voltage_time_sel,
-	    NULL, bd718x7_set_buck_uvp, bd718x7_set_buck_ovp);
+	    NULL, bd718x7_set_buck_uvp, bd718x7_set_buck_ovp, bd717x7_get_buck_prot);
 
 BD718XX_OPS(bd71837_buck_regulator_nolinear_ops, regulator_list_voltage_table,
 	    regulator_map_voltage_ascend, rohm_regulator_set_voltage_sel_restricted,
 	    regulator_get_voltage_sel_regmap, regulator_set_voltage_time_sel,
-	    NULL, bd718x7_set_buck_uvp, bd718x7_set_buck_ovp);
+	    NULL, bd718x7_set_buck_uvp, bd718x7_set_buck_ovp, bd717x7_get_buck_prot);
 /*
  * BD71837 bucks 3 and 4 support defining their enable/disable state also
  * when buck enable state is under HW state machine control. In that case the
@@ -662,7 +582,7 @@ BD718XX_OPS(bd718xx_dvs_buck_regulator_ops, regulator_list_voltage_linear_range,
 	    NULL, regulator_set_voltage_sel_regmap,
 	    regulator_get_voltage_sel_regmap, regulator_set_voltage_time_sel,
 	    regulator_set_ramp_delay_regmap, bd718x7_set_buck_uvp,
-	    bd718x7_set_buck_ovp);
+	    bd718x7_set_buck_ovp, bd717x7_get_buck_prot);
 
 
 
@@ -1772,6 +1692,12 @@ static int bd718xx_probe(struct platform_device *pdev)
 		else
 			desc->ops = swops[i];
 
+		/*
+		 * bd718x7 requires to disable a regulator's monitors while it
+		 * changes to a higher value.
+		 */
+		desc->mon_disable_reg_set_higher = 1;
+
 		rdev = devm_regulator_register(&pdev->dev, desc, &config);
 		if (IS_ERR(rdev))
 			return dev_err_probe(&pdev->dev, PTR_ERR(rdev),

-- 
2.34.1

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ