| Message ID | 20240907065043.771364-7-aardelean@baylibre.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: adc: ad7606: add support for AD7606C-{16,18} parts | expand |
On Sat, 7 Sep 2024 09:50:39 +0300 Alexandru Ardelean <aardelean@baylibre.com> wrote: > For SW mode, the oversampling and scales attributes are always present. > So, they can be implemented via a 'read_avail' hook in iio_info. > > For HW mode, it's a bit tricky, as these attributes get assigned based on > GPIO definitions. > > So, for SW mode, we define a separate AD7606_SW_CHANNEL() macro, and use > that for the SW channels. > And 'ad7606_info_os_range_and_debug' can be renamed to > 'ad7606_info_sw_mode' as it is only used for SW mode. > > For the 'read_avail' hook, we'll need to allocate the SW scales, so that > they are just returned userspace without any extra processing. > The allocation will happen when then ad7606_state struct is allocated. > The oversampling available parameters don't need any extra processing; they > can just be passed back to userspace (as they are). > > Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com> One question inline. > > int ad7606_probe(struct device *dev, int irq, void __iomem *base_address, > diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h > index 635407c2acc0..fa175cff256c 100644 > --- a/drivers/iio/adc/ad7606.h > +++ b/drivers/iio/adc/ad7606.h > #define AD7616_CHANNEL(num) AD7606_SW_CHANNEL(num, 16) > > /** > @@ -65,11 +84,15 @@ struct ad7606_chip_info { > /** > * struct ad7606_chan_scale - channel scale configuration > * @scale_avail pointer to the array which stores the available scales > + * @scale_avail_show a duplicate of 'scale_avail' which is readily formatted > + * such that it can be read via the 'read_avail' hook > * @num_scales number of elements stored in the scale_avail array > * @range voltage range selection, selects which scale to apply > */ > struct ad7606_chan_scale { > +#define AD760X_MAX_SCALE_SHOW (AD760X_MAX_CHANNELS * 2) Why is the number of scales dependent on the number of channels? > const unsigned int *scale_avail; > + int scale_avail_show[AD760X_MAX_SCALE_SHOW]; > unsigned int num_scales; > unsigned int range; > };
On Sat, Sep 7, 2024 at 6:04 PM Jonathan Cameron <jic23@kernel.org> wrote: > > On Sat, 7 Sep 2024 09:50:39 +0300 > Alexandru Ardelean <aardelean@baylibre.com> wrote: > > > For SW mode, the oversampling and scales attributes are always present. > > So, they can be implemented via a 'read_avail' hook in iio_info. > > > > For HW mode, it's a bit tricky, as these attributes get assigned based on > > GPIO definitions. > > > > So, for SW mode, we define a separate AD7606_SW_CHANNEL() macro, and use > > that for the SW channels. > > And 'ad7606_info_os_range_and_debug' can be renamed to > > 'ad7606_info_sw_mode' as it is only used for SW mode. > > > > For the 'read_avail' hook, we'll need to allocate the SW scales, so that > > they are just returned userspace without any extra processing. > > The allocation will happen when then ad7606_state struct is allocated. > > The oversampling available parameters don't need any extra processing; they > > can just be passed back to userspace (as they are). > > > > Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com> > > One question inline. > > > > > int ad7606_probe(struct device *dev, int irq, void __iomem *base_address, > > diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h > > index 635407c2acc0..fa175cff256c 100644 > > --- a/drivers/iio/adc/ad7606.h > > +++ b/drivers/iio/adc/ad7606.h > > > #define AD7616_CHANNEL(num) AD7606_SW_CHANNEL(num, 16) > > > > /** > > @@ -65,11 +84,15 @@ struct ad7606_chip_info { > > /** > > * struct ad7606_chan_scale - channel scale configuration > > * @scale_avail pointer to the array which stores the available scales > > + * @scale_avail_show a duplicate of 'scale_avail' which is readily formatted > > + * such that it can be read via the 'read_avail' hook > > * @num_scales number of elements stored in the scale_avail array > > * @range voltage range selection, selects which scale to apply > > */ > > struct ad7606_chan_scale { > > +#define AD760X_MAX_SCALE_SHOW (AD760X_MAX_CHANNELS * 2) > > Why is the number of scales dependent on the number of channels? Well, that was just a lazy implementation. I doubt there would be 16 scales (channel range values) for any newer part. A value for a channel-range is 4-bits (for 0 to 15). In IIO the current highest scale-count is 5; I guess I can update this to a macro, and use that. > > > const unsigned int *scale_avail; > > + int scale_avail_show[AD760X_MAX_SCALE_SHOW]; > > unsigned int num_scales; > > unsigned int range; > > }; >
On Sat, 7 Sep 2024 21:10:10 +0300 Alexandru Ardelean <aardelean@baylibre.com> wrote: > On Sat, Sep 7, 2024 at 6:04 PM Jonathan Cameron <jic23@kernel.org> wrote: > > > > On Sat, 7 Sep 2024 09:50:39 +0300 > > Alexandru Ardelean <aardelean@baylibre.com> wrote: > > > > > For SW mode, the oversampling and scales attributes are always present. > > > So, they can be implemented via a 'read_avail' hook in iio_info. > > > > > > For HW mode, it's a bit tricky, as these attributes get assigned based on > > > GPIO definitions. > > > > > > So, for SW mode, we define a separate AD7606_SW_CHANNEL() macro, and use > > > that for the SW channels. > > > And 'ad7606_info_os_range_and_debug' can be renamed to > > > 'ad7606_info_sw_mode' as it is only used for SW mode. > > > > > > For the 'read_avail' hook, we'll need to allocate the SW scales, so that > > > they are just returned userspace without any extra processing. > > > The allocation will happen when then ad7606_state struct is allocated. > > > The oversampling available parameters don't need any extra processing; they > > > can just be passed back to userspace (as they are). > > > > > > Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com> > > > > One question inline. > > > > > > > > int ad7606_probe(struct device *dev, int irq, void __iomem *base_address, > > > diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h > > > index 635407c2acc0..fa175cff256c 100644 > > > --- a/drivers/iio/adc/ad7606.h > > > +++ b/drivers/iio/adc/ad7606.h > > > > > #define AD7616_CHANNEL(num) AD7606_SW_CHANNEL(num, 16) > > > > > > /** > > > @@ -65,11 +84,15 @@ struct ad7606_chip_info { > > > /** > > > * struct ad7606_chan_scale - channel scale configuration > > > * @scale_avail pointer to the array which stores the available scales > > > + * @scale_avail_show a duplicate of 'scale_avail' which is readily formatted > > > + * such that it can be read via the 'read_avail' hook > > > * @num_scales number of elements stored in the scale_avail array > > > * @range voltage range selection, selects which scale to apply > > > */ > > > struct ad7606_chan_scale { > > > +#define AD760X_MAX_SCALE_SHOW (AD760X_MAX_CHANNELS * 2) > > > > Why is the number of scales dependent on the number of channels? > > Well, that was just a lazy implementation. > I doubt there would be 16 scales (channel range values) for any newer part. > A value for a channel-range is 4-bits (for 0 to 15). > > In IIO the current highest scale-count is 5; I guess I can update this > to a macro, and use that. Indeed defining this as 16 would make more sense than basing it on random unconnected thing! Jonathan > > > > > > > const unsigned int *scale_avail; > > > + int scale_avail_show[AD760X_MAX_SCALE_SHOW]; > > > unsigned int num_scales; > > > unsigned int range; > > > }; > >
diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c index 39ef72a8acd2..364f16fb96bf 100644 --- a/drivers/iio/adc/ad7606.c +++ b/drivers/iio/adc/ad7606.c @@ -483,6 +483,37 @@ static int ad7606_buffer_predisable(struct iio_dev *indio_dev) return 0; } +static int ad7606_read_avail(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + const int **vals, int *type, int *length, + long info) +{ + struct ad7606_state *st = iio_priv(indio_dev); + struct ad7606_chan_scale *cs; + unsigned int ch = 0; + + switch (info) { + case IIO_CHAN_INFO_OVERSAMPLING_RATIO: + *vals = st->oversampling_avail; + *length = st->num_os_ratios; + *type = IIO_VAL_INT; + + return IIO_AVAIL_LIST; + + case IIO_CHAN_INFO_SCALE: + if (st->sw_mode_en) + ch = chan->address; + + cs = &st->chan_scales[ch]; + *vals = cs->scale_avail_show; + *length = cs->num_scales * 2; + *type = IIO_VAL_INT_PLUS_MICRO; + + return IIO_AVAIL_LIST; + } + return -EINVAL; +} + static const struct iio_buffer_setup_ops ad7606_buffer_ops = { .postenable = &ad7606_buffer_postenable, .predisable = &ad7606_buffer_predisable, @@ -500,11 +531,11 @@ static const struct iio_info ad7606_info_os_and_range = { .validate_trigger = &ad7606_validate_trigger, }; -static const struct iio_info ad7606_info_os_range_and_debug = { +static const struct iio_info ad7606_info_sw_mode = { .read_raw = &ad7606_read_raw, .write_raw = &ad7606_write_raw, + .read_avail = &ad7606_read_avail, .debugfs_reg_access = &ad7606_reg_access, - .attrs = &ad7606_attribute_group_os_and_range, .validate_trigger = &ad7606_validate_trigger, }; @@ -530,7 +561,7 @@ static int ad7606_sw_mode_setup(struct iio_dev *indio_dev) { unsigned int num_channels = indio_dev->num_channels - 1; struct ad7606_state *st = iio_priv(indio_dev); - int ch; + int ret, ch; if (!st->bops->sw_mode_config) return 0; @@ -539,7 +570,7 @@ static int ad7606_sw_mode_setup(struct iio_dev *indio_dev) if (!st->sw_mode_en) return 0; - indio_dev->info = &ad7606_info_os_range_and_debug; + indio_dev->info = &ad7606_info_sw_mode; /* Scale of 0.076293 is only available in sw mode */ /* After reset, in software mode, ±10 V is set by default */ @@ -551,7 +582,29 @@ static int ad7606_sw_mode_setup(struct iio_dev *indio_dev) cs->range = 2; } - return st->bops->sw_mode_config(indio_dev); + ret = st->bops->sw_mode_config(indio_dev); + if (ret) + return ret; + + for (ch = 0; ch < num_channels; ch++) { + struct ad7606_chan_scale *cs = &st->chan_scales[ch]; + int i; + + cs = &st->chan_scales[ch]; + + if (cs->num_scales * 2 > AD760X_MAX_SCALE_SHOW) { + dev_err(st->dev, "Driver error: scale range too big"); + return -ERANGE; + } + + /* Generate a scale_avail list for showing to userspace */ + for (i = 0; i < cs->num_scales; i++) { + cs->scale_avail_show[i * 2] = 0; + cs->scale_avail_show[i * 2 + 1] = cs->scale_avail[i]; + } + } + + return 0; } int ad7606_probe(struct device *dev, int irq, void __iomem *base_address, diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h index 635407c2acc0..fa175cff256c 100644 --- a/drivers/iio/adc/ad7606.h +++ b/drivers/iio/adc/ad7606.h @@ -27,6 +27,29 @@ }, \ } +#define AD7606_SW_CHANNEL(num, bits) { \ + .type = IIO_VOLTAGE, \ + .indexed = 1, \ + .channel = num, \ + .address = num, \ + .info_mask_separate = \ + BIT(IIO_CHAN_INFO_RAW) | \ + BIT(IIO_CHAN_INFO_SCALE), \ + .info_mask_separate_available = \ + BIT(IIO_CHAN_INFO_SCALE), \ + .info_mask_shared_by_all = \ + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \ + .info_mask_shared_by_all_available = \ + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \ + .scan_index = num, \ + .scan_type = { \ + .sign = 's', \ + .realbits = (bits), \ + .storagebits = (bits) > 16 ? 32 : 16, \ + .endianness = IIO_CPU, \ + }, \ +} + #define AD7605_CHANNEL(num) \ AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW), \ BIT(IIO_CHAN_INFO_SCALE), 0, 16) @@ -36,10 +59,6 @@ BIT(IIO_CHAN_INFO_SCALE), \ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), bits) -#define AD7606_SW_CHANNEL(num, bits) \ - AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),\ - 0, BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), bits) - #define AD7616_CHANNEL(num) AD7606_SW_CHANNEL(num, 16) /** @@ -65,11 +84,15 @@ struct ad7606_chip_info { /** * struct ad7606_chan_scale - channel scale configuration * @scale_avail pointer to the array which stores the available scales + * @scale_avail_show a duplicate of 'scale_avail' which is readily formatted + * such that it can be read via the 'read_avail' hook * @num_scales number of elements stored in the scale_avail array * @range voltage range selection, selects which scale to apply */ struct ad7606_chan_scale { +#define AD760X_MAX_SCALE_SHOW (AD760X_MAX_CHANNELS * 2) const unsigned int *scale_avail; + int scale_avail_show[AD760X_MAX_SCALE_SHOW]; unsigned int num_scales; unsigned int range; };
For SW mode, the oversampling and scales attributes are always present. So, they can be implemented via a 'read_avail' hook in iio_info. For HW mode, it's a bit tricky, as these attributes get assigned based on GPIO definitions. So, for SW mode, we define a separate AD7606_SW_CHANNEL() macro, and use that for the SW channels. And 'ad7606_info_os_range_and_debug' can be renamed to 'ad7606_info_sw_mode' as it is only used for SW mode. For the 'read_avail' hook, we'll need to allocate the SW scales, so that they are just returned userspace without any extra processing. The allocation will happen when then ad7606_state struct is allocated. The oversampling available parameters don't need any extra processing; they can just be passed back to userspace (as they are). Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com> --- drivers/iio/adc/ad7606.c | 63 ++++++++++++++++++++++++++++++++++++---- drivers/iio/adc/ad7606.h | 31 +++++++++++++++++--- 2 files changed, 85 insertions(+), 9 deletions(-)