diff mbox series

[v2,10/10] iio: adc: ad7606: Disable PWM usage for non backend version

Message ID 20240920-ad7606_add_iio_backend_support-v2-10-0e78782ae7d0@baylibre.com
State Handled Elsewhere
Headers show
Series Add iio backend compatibility for ad7606 | expand

Commit Message

Guillaume Stols Sept. 20, 2024, 5:33 p.m. UTC
Since the pwm was introduced before backend, there was an example use
whit triggered buffers. However, using it may be dangerous, because if
the PWM goes too fast, a new conversion can be triggered before the
transmission is over, whit the associated mess in the buffers.
Until a solution is found to mitigate this risk, for instante CRC
support, the PWM will be disabled.

Signed-off-by: Guillaume Stols <gstols@baylibre.com>
---
 drivers/iio/adc/ad7606.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

David Lechner Sept. 23, 2024, 8:34 a.m. UTC | #1
On Fri, Sep 20, 2024 at 7:33 PM Guillaume Stols <gstols@baylibre.com> wrote:
>
> Since the pwm was introduced before backend, there was an example use
> whit triggered buffers. However, using it may be dangerous, because if

with

> the PWM goes too fast, a new conversion can be triggered before the
> transmission is over, whit the associated mess in the buffers.

with

> Until a solution is found to mitigate this risk, for instante CRC

instance

> support, the PWM will be disabled.
>
> Signed-off-by: Guillaume Stols <gstols@baylibre.com>
> ---
Jonathan Cameron Sept. 29, 2024, 12:56 p.m. UTC | #2
On Fri, 20 Sep 2024 17:33:30 +0000
Guillaume Stols <gstols@baylibre.com> wrote:

> Since the pwm was introduced before backend, there was an example use
> whit triggered buffers. However, using it may be dangerous, because if
> the PWM goes too fast, a new conversion can be triggered before the
> transmission is over, whit the associated mess in the buffers.
> Until a solution is found to mitigate this risk, for instante CRC
> support, the PWM will be disabled.
> 
> Signed-off-by: Guillaume Stols <gstols@baylibre.com>

Whilst I can see the logic introducing it in two steps, maybe
shout a bit more about that in the original patch as otherwise
some foolish unnamed maintainer might pick up part of this series
leaving things in a bad state for a kernel release.

The earlier patch 'recommends' it isn't used which is a bit week.
Say support is going to be removed later.
> ---
>  drivers/iio/adc/ad7606.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
> index f710445bdc22..0c12ca237ee9 100644
> --- a/drivers/iio/adc/ad7606.c
> +++ b/drivers/iio/adc/ad7606.c
> @@ -599,7 +599,6 @@ static int ad7606_buffer_postenable(struct iio_dev *indio_dev)
>  	struct ad7606_state *st = iio_priv(indio_dev);
>  
>  	gpiod_set_value(st->gpio_convst, 1);
> -	ad7606_pwm_set_swing(st);
>  
>  	return 0;
>  }
> @@ -609,7 +608,6 @@ static int ad7606_buffer_predisable(struct iio_dev *indio_dev)
>  	struct ad7606_state *st = iio_priv(indio_dev);
>  
>  	gpiod_set_value(st->gpio_convst, 0);
> -	ad7606_pwm_set_low(st);
>  
>  	return 0;
>  }
> @@ -838,6 +836,12 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
>  		indio_dev->setup_ops = &ad7606_pwm_buffer_ops;
>  	} else {
>  		init_completion(&st->completion);
> +
> +		/* Reserve the PWM use only for backend (force gpio_convst definition) */
> +		if (!st->gpio_convst)
> +			return dev_err_probe(dev, -EINVAL,
> +					     "No backend, connect convst to a GPIO");
> +
>  		ret = devm_request_threaded_irq(dev, irq,
>  						NULL,
>  						&ad7606_interrupt,
>
diff mbox series

Patch

diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
index f710445bdc22..0c12ca237ee9 100644
--- a/drivers/iio/adc/ad7606.c
+++ b/drivers/iio/adc/ad7606.c
@@ -599,7 +599,6 @@  static int ad7606_buffer_postenable(struct iio_dev *indio_dev)
 	struct ad7606_state *st = iio_priv(indio_dev);
 
 	gpiod_set_value(st->gpio_convst, 1);
-	ad7606_pwm_set_swing(st);
 
 	return 0;
 }
@@ -609,7 +608,6 @@  static int ad7606_buffer_predisable(struct iio_dev *indio_dev)
 	struct ad7606_state *st = iio_priv(indio_dev);
 
 	gpiod_set_value(st->gpio_convst, 0);
-	ad7606_pwm_set_low(st);
 
 	return 0;
 }
@@ -838,6 +836,12 @@  int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
 		indio_dev->setup_ops = &ad7606_pwm_buffer_ops;
 	} else {
 		init_completion(&st->completion);
+
+		/* Reserve the PWM use only for backend (force gpio_convst definition) */
+		if (!st->gpio_convst)
+			return dev_err_probe(dev, -EINVAL,
+					     "No backend, connect convst to a GPIO");
+
 		ret = devm_request_threaded_irq(dev, irq,
 						NULL,
 						&ad7606_interrupt,