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

Jonathan Cameron
2024-09-29T12:56:15Z

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,
>