* [PATCH v2] iio: dac: ad7293: enable power before reset
@ 2024-12-16 21:44 David Lechner
2024-12-17 10:05 ` Nuno Sá
0 siblings, 1 reply; 3+ messages in thread
From: David Lechner @ 2024-12-16 21:44 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Michael Hennerich, Liam Girdwood, Mark Brown, Antoniu Miclaus,
Nuno Sá, linux-iio, linux-kernel, David Lechner
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@baylibre.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@baylibre.com>
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] iio: dac: ad7293: enable power before reset
2024-12-16 21:44 [PATCH v2] iio: dac: ad7293: enable power before reset David Lechner
@ 2024-12-17 10:05 ` Nuno Sá
2024-12-19 16:37 ` Jonathan Cameron
0 siblings, 1 reply; 3+ messages in thread
From: Nuno Sá @ 2024-12-17 10:05 UTC (permalink / raw)
To: David Lechner, Jonathan Cameron
Cc: Michael Hennerich, Liam Girdwood, Mark Brown, Antoniu Miclaus,
Nuno Sá, linux-iio, linux-kernel
On Mon, 2024-12-16 at 15:44 -0600, David Lechner wrote:
> 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@baylibre.com>
> ---
Reviewed-by: Nuno Sa <nuno.sa@analog.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..d3f49b5337d2f512363d50b434d99d4e9b95
> 059f 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,
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] iio: dac: ad7293: enable power before reset
2024-12-17 10:05 ` Nuno Sá
@ 2024-12-19 16:37 ` Jonathan Cameron
0 siblings, 0 replies; 3+ messages in thread
From: Jonathan Cameron @ 2024-12-19 16:37 UTC (permalink / raw)
To: Nuno Sá
Cc: David Lechner, Michael Hennerich, Liam Girdwood, Mark Brown,
Antoniu Miclaus, Nuno Sá, linux-iio, linux-kernel
On Tue, 17 Dec 2024 11:05:38 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:
> On Mon, 2024-12-16 at 15:44 -0600, David Lechner wrote:
> > 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@baylibre.com>
> > ---
>
> Reviewed-by: Nuno Sa <nuno.sa@analog.com>
Applied. Thanks
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-12-19 16:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-16 21:44 [PATCH v2] iio: dac: ad7293: enable power before reset David Lechner
2024-12-17 10:05 ` Nuno Sá
2024-12-19 16:37 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox