...
> >>
> >> ret = ad7606_read_samples(st);
> >> @@ -271,6 +284,12 @@ static int ad7606_read_raw(struct iio_dev *indio_dev,
> >> case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> >> *val = st->oversampling;
> >> return IIO_VAL_INT;
> >> + case IIO_CHAN_INFO_SAMP_FREQ:
> >> + pwm_get_state_hw(st->cnvst_pwm, &cnvst_pwm_state);
> >> + /* If the PWM is swinging, return the real frequency, otherwise 0 */
> > So this only exists for the pwm case. In that case can we split the channel definitions
> > into versions with an without this and register just the right one.
> >
> > A sampling frequency of 0 usually means no sampling, not that we can tell what it
> > is. If we can't tell don't provide the file.
>
> The file is provided only for the "backended" device
> (AD7606_BI_CHANNEL), BI being Backend Interface. This mode only works
> with PWM (and incidentally PWM is meant to be used only in conjuction
> with backend).
>
> When the PWM is not running because e.g sampling is not enabled, or PWM
> failed to start, I return 0. Shall I always return the configured value
> instead of the real one ?
Yes. Configured should be fine I think if there is no way to ask
'what will it be when I turn it on'.
> >> diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h
> >> index aab8fefb84be..9a098cd77812 100644
> >> --- a/drivers/iio/adc/ad7606.h
> >> +++ b/drivers/iio/adc/ad7606.h
> >> @@ -34,6 +34,12 @@
> >> BIT(IIO_CHAN_INFO_SCALE), \
> >> BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO))
> >>
> >> +#define AD7606_BI_CHANNEL(num) \
> >> + AD760X_CHANNEL(num, 0, \
> >> + BIT(IIO_CHAN_INFO_SCALE), \
> >> + BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
> >> + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO))
> >> +
> >> #define AD7616_CHANNEL(num) \
> >> AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),\
> >> 0, BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO))
> >> @@ -61,6 +67,7 @@ enum ad7606_supported_device_ids {
> >> * @os_req_reset some devices require a reset to update oversampling
> >> * @init_delay_ms required delay in miliseconds for initialization
> >> * after a restart
> >> + * @has_backend defines if a backend is available for the given chip
> > That seems to me more of a case of does the driver support it.
> > Linux kernel code has no way of knowing if a backend hardware exists
> > or not. Modify the comment to speak about if we know it works.
> >
> > Or is there something fundamental that stops the backend approach
> > working with some devices?
> >
> > Why does the driver need this flag?
>
> Potentially, I think any of those parts can have a backend and moreover,
> I don't see anything preventing any ADC to have a backend.
>
> I introduced the flag as a way to differentiate the "new" way of
> supporting parallel interface, i.e using backend, from the "old" way
> (using port I/O).
>
> There is a concurrency between the old implementation using port I/O and
> the new one using iio-backend, because they are both "platform", so the
> initial idea was that it would not make sense and be dangerous to look
> for a backend for the parts that have no existing (i'd rather say, like
> you pointed out, supported) backend.
>
> Having a second thought at it, the dt bindings already permits only
> io-backend property to be populated for the parts that actually have a
> backend, hence one of these is superfluous, or maybe even both are and
> the user is responsible for setting the right value in the dts. Any advice ?
Dt binding should be enough. The worst that happens is the driver
tries to use an unsupported backend and that will fail I hope.
So I wouldn't have this driver try to stop it.
> >> diff --git a/drivers/iio/adc/ad7606_par.c b/drivers/iio/adc/ad7606_par.c
> >> index d83c0edc1e31..5c8a04556e25 100644
> >> --- a/drivers/iio/adc/ad7606_par.c
> >> +++ b/drivers/iio/adc/ad7606_par.c
> >> @@ -102,3 +195,6 @@ MODULE_AUTHOR("Michael Hennerich <michael.hennerich@xxxxxxxxxx>");
> >> MODULE_DESCRIPTION("Analog Devices AD7606 ADC");
> >> MODULE_LICENSE("GPL v2");
> >> MODULE_IMPORT_NS(IIO_AD7606);
> >> +#ifdef CONFIG_IIO_BACKEND
> >> +MODULE_IMPORT_NS(IIO_BACKEND);
> > I'd not bother with config guards. Importing a namespace we don't
> > use should be harmless.
> OK, will remove it. According to Nuno's feedback, I could also force the
> selection of CONFIG_IIO_BACKEND with the driver, which IMHO is not a bad
> idea, as it would allow to remove all those ifdefs.
Hmm. I guess the questions is whether that is a bloat anyone will worry about
who is using the old way for this device. I guess that's a problem for
Analog folk if their customers complain. We can always relax this in future
so for now select IIO_BACKEND is fine by me.
Jonathan
