linux-kernel - [PATCH v2] iio: dac: ad7293: enable power before reset

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-next>] [day] [month] [year] [list]

Message-Id: <20241216-iio-regulator-cleanup-round-6-v2-1-9482164b68cb@baylibre.com>
Date: Mon, 16 Dec 2024 15:44:03 -0600
From: David Lechner <dlechner@...libre.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: Michael Hennerich <Michael.Hennerich@...log.com>, 
 Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>, 
 Antoniu Miclaus <antoniu.miclaus@...log.com>, 
 Nuno Sá <nuno.sa@...log.com>, linux-iio@...r.kernel.org, 
 linux-kernel@...r.kernel.org, David Lechner <dlechner@...libre.com>
Subject: [PATCH v2] iio: dac: ad7293: enable power before reset

Change the order of regulator enable and reset so that power supplies
are turned on before touching the reset line. Generally, chips should
have the VDRIVE supply enabled before applying voltage on any pins.

While we are at it, remove the voltage level checks. If the power
supplies are not supplying the correct voltage, this is a hardware
design problem, not a software problem.

Signed-off-by: David Lechner <dlechner@...libre.com>
---
Changes in v2:
- Dropped patches from "iio: use devm_regulator_get_enable_read_voltage
  round 6" that have already been applied.
- New patch for ad7293 that just enables power supplies and no longer
  reads the voltage.
- Link to v1: https://lore.kernel.org/r/20241120-iio-regulator-cleanup-round-6-v1-0-d5a5360f7ec3@baylibre.com
---
 drivers/iio/dac/ad7293.c | 68 +++++++-----------------------------------------
 1 file changed, 9 insertions(+), 59 deletions(-)

diff --git a/drivers/iio/dac/ad7293.c b/drivers/iio/dac/ad7293.c
index 1d403267048240be1bf3d8b2a59685409b9087fd..d3f49b5337d2f512363d50b434d99d4e9b95059f 100644
--- a/drivers/iio/dac/ad7293.c
+++ b/drivers/iio/dac/ad7293.c
@@ -141,8 +141,6 @@ struct ad7293_state {
 	/* Protect against concurrent accesses to the device, page selection and data content */
 	struct mutex lock;
 	struct gpio_desc *gpio_reset;
-	struct regulator *reg_avdd;
-	struct regulator *reg_vdrive;
 	u8 page_select;
 	u8 data[3] __aligned(IIO_DMA_MINALIGN);
 };
@@ -777,6 +775,15 @@ static int ad7293_reset(struct ad7293_state *st)
 static int ad7293_properties_parse(struct ad7293_state *st)
 {
 	struct spi_device *spi = st->spi;
+	int ret;
+
+	ret = devm_regulator_get_enable(&spi->dev, "avdd");
+	if (ret)
+		return dev_err_probe(&spi->dev, ret, "failed to enable AVDD\n");
+
+	ret = devm_regulator_get_enable(&spi->dev, "vdrive");
+	if (ret)
+		return dev_err_probe(&spi->dev, ret, "failed to enable VDRIVE\n");
 
 	st->gpio_reset = devm_gpiod_get_optional(&st->spi->dev, "reset",
 						 GPIOD_OUT_HIGH);
@@ -784,24 +791,9 @@ static int ad7293_properties_parse(struct ad7293_state *st)
 		return dev_err_probe(&spi->dev, PTR_ERR(st->gpio_reset),
 				     "failed to get the reset GPIO\n");
 
-	st->reg_avdd = devm_regulator_get(&spi->dev, "avdd");
-	if (IS_ERR(st->reg_avdd))
-		return dev_err_probe(&spi->dev, PTR_ERR(st->reg_avdd),
-				     "failed to get the AVDD voltage\n");
-
-	st->reg_vdrive = devm_regulator_get(&spi->dev, "vdrive");
-	if (IS_ERR(st->reg_vdrive))
-		return dev_err_probe(&spi->dev, PTR_ERR(st->reg_vdrive),
-				     "failed to get the VDRIVE voltage\n");
-
 	return 0;
 }
 
-static void ad7293_reg_disable(void *data)
-{
-	regulator_disable(data);
-}
-
 static int ad7293_init(struct ad7293_state *st)
 {
 	int ret;
@@ -816,48 +808,6 @@ static int ad7293_init(struct ad7293_state *st)
 	if (ret)
 		return ret;
 
-	ret = regulator_enable(st->reg_avdd);
-	if (ret) {
-		dev_err(&spi->dev,
-			"Failed to enable specified AVDD Voltage!\n");
-		return ret;
-	}
-
-	ret = devm_add_action_or_reset(&spi->dev, ad7293_reg_disable,
-				       st->reg_avdd);
-	if (ret)
-		return ret;
-
-	ret = regulator_enable(st->reg_vdrive);
-	if (ret) {
-		dev_err(&spi->dev,
-			"Failed to enable specified VDRIVE Voltage!\n");
-		return ret;
-	}
-
-	ret = devm_add_action_or_reset(&spi->dev, ad7293_reg_disable,
-				       st->reg_vdrive);
-	if (ret)
-		return ret;
-
-	ret = regulator_get_voltage(st->reg_avdd);
-	if (ret < 0) {
-		dev_err(&spi->dev, "Failed to read avdd regulator: %d\n", ret);
-		return ret;
-	}
-
-	if (ret > 5500000 || ret < 4500000)
-		return -EINVAL;
-
-	ret = regulator_get_voltage(st->reg_vdrive);
-	if (ret < 0) {
-		dev_err(&spi->dev,
-			"Failed to read vdrive regulator: %d\n", ret);
-		return ret;
-	}
-	if (ret > 5500000 || ret < 1700000)
-		return -EINVAL;
-
 	/* Check Chip ID */
 	ret = __ad7293_spi_read(st, AD7293_REG_DEVICE_ID, &chip_id);
 	if (ret)

---
base-commit: 01958cb8a00d9721ae56ad1eef9cd7b22b5a34bb
change-id: 20241120-iio-regulator-cleanup-round-6-78b05be06718

Best regards,
-- 
David Lechner <dlechner@...libre.com>