| Message ID | 20240501-adding-new-ad738x-driver-v6-9-3c0741154728@baylibre.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: adc: add new ad7380 driver | expand |
On Wed, 2024-05-01 at 16:55 +0200, Julien Stephan wrote: > Adds support for rolling average oversampling mode. > > Rolling oversampling mode uses a first in, first out (FIFO) buffer of > the most recent samples in the averaging calculation, allowing the ADC > throughput rate and output data rate to stay the same, since we only need > to take only one sample for each new conversion. > > The FIFO length is 8, thus the available oversampling ratios are 1, 2, 4, 8 > in this mode (vs 1, 2, 4, 8, 16, 32 for the normal average) > > In order to be able to change the averaging mode, this commit also adds > the new "oversampling_mode" and "oversampling_mode_available" custom > attributes along with the according documentation file in > Documentation/ABI/testing/sysfs-bus-iio-adc-ad7380 since no standard > attributes correspond to this use case. > > Signed-off-by: Julien Stephan <jstephan@baylibre.com> > --- > Documentation/ABI/testing/sysfs-bus-iio-adc-ad7380 | 38 ++++++ > MAINTAINERS | 1 + > drivers/iio/adc/ad7380.c | 143 +++++++++++++++++++-- > 3 files changed, 174 insertions(+), 8 deletions(-) > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-ad7380 > b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad7380 > new file mode 100644 > index 000000000000..0a560ef3e32a > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad7380 > @@ -0,0 +1,38 @@ > +What: /sys/bus/iio/devices/iio:deviceX/oversampling_mode > +KernelVersion: 6.9 > +Contact: Michael Hennerich <michael.hennerich@analog.com> > +Description: > + Writing this attribute sets the oversampling average mode. > + Reading it, shows the configured mode. > + Available modes can be displayed using the oversampling_mode_available > + attribute. > + When writing this attribute to change the oversampling mode, this will > + have the following side effects: > + > + - soft reset the ADC to flush the oversampling block and FIFO > + > + - the available oversampling ratios depend on the oversampling mode > + configured so to avoid misconfiguration, changing the mode will disable > + the oversampling by setting the ratio to 1. Hmm, can we somehow re-enable it again with a proper configuration (after the mode change)? > + > + - the list of available ratios (displayed by reading the > + oversampling_ratio_available attribute) will be updated when changing > + the oversampling mode. > + > +What: /sys/bus/iio/devices/iio:deviceX/oversampling_mode_available > +KernelVersion: 6.9 > +Contact: Michael Hennerich <michael.hennerich@analog.com> > +Description: > + Display the available oversampling average modes. The two available modes > + are "normal" and "rolling" where "normal" average mode is the default one. > + > + - normal averaging involves taking a number of samples, adding them > + together, and dividing the result by the number of samples taken. > + This result is then output from the device. The sample data is cleared > + when the process completes. Because we need more samples to output a > + value, the data output rate decrease with the oversampling ratio. > + > + - rolling oversampling mode uses a first in, first out (FIFO) buffer of > + the most recent samples in the averaging calculation, allowing the ADC > + throughput rate and output data rate to stay the same, since we only need > + to take only one sample for each new conversion. > diff --git a/MAINTAINERS b/MAINTAINERS > index 87724a9e9f9f..ca1e115f2aff 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -434,6 +434,7 @@ R: David Lechner <dlechner@baylibre.com> > S: Supported > W: > https://wiki.analog.com/resources/tools-software/linux-drivers/iio-adc/ad738x > W: https://ez.analog.com/linux-software-drivers > +F: Documentation/ABI/testing/sysfs-bus-iio-adc-ad7380 > F: Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml > F: drivers/iio/adc/ad7380.c > > diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c > index 1e3869f5e48c..7b021bb9cf87 100644 > --- a/drivers/iio/adc/ad7380.c > +++ b/drivers/iio/adc/ad7380.c > @@ -51,6 +51,8 @@ > #define AD7380_REG_ADDR_ALERT_HIGH_TH 0x5 > > #define AD7380_CONFIG1_OS_MODE BIT(9) > +#define OS_MODE_NORMAL_AVERAGE 0 > +#define OS_MODE_ROLLING_AVERAGE 1 no AD7380 prefix? > #define AD7380_CONFIG1_OSR GENMASK(8, 6) > #define AD7380_CONFIG1_CRC_W BIT(5) > #define AD7380_CONFIG1_CRC_R BIT(4) > @@ -159,16 +161,27 @@ static const struct ad7380_timing_specs ad7380_4_timing = { > .t_csh_ns = 20, > }; > ... > > +static ssize_t oversampling_mode_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t len) > +{ > + struct iio_dev *indio_dev = dev_to_iio_dev(dev); > + struct ad7380_state *st = iio_priv(indio_dev); > + int os_mode, ret; > + > + ret = sysfs_match_string(ad7380_oversampling_average_modes, buf); > + if (ret < 0) > + return ret; > + > + os_mode = ret; > + > + iio_device_claim_direct_scoped(return -EBUSY, indio_dev) { > + ret = regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG1, > + AD7380_CONFIG1_OS_MODE, > + FIELD_PREP(AD7380_CONFIG1_OS_MODE, > os_mode)); > + > + if (ret) > + return ret; > + > + st->oversampling_mode = os_mode; > + > + /* > + * Oversampling ratio depends on oversampling mode, to avoid > + * misconfiguration when changing oversampling mode, > + * disable oversampling by setting OSR to 0. > + */ Given the comment, I would expect you to disable osr before changing the mode. > + ret = regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG1, > + AD7380_CONFIG1_OSR, > FIELD_PREP(AD7380_CONFIG1_OSR, 0)); > + > + if (ret) > + return ret; > + > + st->oversampling_ratio = 1; > + 1? Also, it would be nice if we could re-enabled osr. So the flow would maybe be: 1) disable osr; 2) change mode; 3) do whatever we need so configuration makes sense for new mode? 4) re-enable osr; 5) soft-reset; Also not sure if 4) and 5) are in the correct order. > + /* > + * Perform a soft reset. > + * This will flush the oversampling block and FIFO but will > + * maintain the content of the configurable registers. > + */ > + ret = regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG2, > + AD7380_CONFIG2_RESET, > + FIELD_PREP(AD7380_CONFIG2_RESET, > + AD7380_CONFIG2_RESET_SOFT)); > + } > + return ret ?: len; > +} > + > +static ssize_t oversampling_mode_available_show(struct device *dev, > + struct device_attribute *attr, > char *buf) > +{ > + int i; > + size_t len = 0; > + > + for (i = 0; i < ARRAY_SIZE(ad7380_oversampling_average_modes); i++) > + len += sysfs_emit_at(buf, len, "%s ", > ad7380_oversampling_average_modes[i]); > + > + buf[len - 1] = '\n'; > + > + return len; > +} > + > +static IIO_DEVICE_ATTR_RW(oversampling_mode, 0); > +static IIO_DEVICE_ATTR_RO(oversampling_mode_available, 0); > + > +static struct attribute *ad7380_attributes[] = { > + &iio_dev_attr_oversampling_mode.dev_attr.attr, > + &iio_dev_attr_oversampling_mode_available.dev_attr.attr, > + NULL > +}; > + Maybe use IIO_ENUM... It allows the core to do some of the stuff you're doing for you. - Nuno Sá
On Wed, 01 May 2024 16:55:42 +0200 Julien Stephan <jstephan@baylibre.com> wrote: > Adds support for rolling average oversampling mode. > > Rolling oversampling mode uses a first in, first out (FIFO) buffer of > the most recent samples in the averaging calculation, allowing the ADC > throughput rate and output data rate to stay the same, since we only need > to take only one sample for each new conversion. > > The FIFO length is 8, thus the available oversampling ratios are 1, 2, 4, 8 > in this mode (vs 1, 2, 4, 8, 16, 32 for the normal average) Ah. I should have read on! > > In order to be able to change the averaging mode, this commit also adds > the new "oversampling_mode" and "oversampling_mode_available" custom > attributes along with the according documentation file in > Documentation/ABI/testing/sysfs-bus-iio-adc-ad7380 since no standard > attributes correspond to this use case. This comes to the comment I stuck in the previous patch. To most people this is not a form of oversampling because the data rate remains unchanged. It's a cheap low pass filter (boxcar) Google pointed me at: https://dsp.stackexchange.com/questions/9966/what-is-the-cut-off-frequency-of-a-moving-average-filter in_voltage_low_pass_3db_frequency would be the most appropriate standard ABI for this if we do treat it as a low pass filter control. I'm not necessarily saying we don't want new ABI for this, but I would like to consider the pros and cons of just using the 3db frequency. So would that work for this part or am I missing something? > > Signed-off-by: Julien Stephan <jstephan@baylibre.com> > --- > Documentation/ABI/testing/sysfs-bus-iio-adc-ad7380 | 38 ++++++ > MAINTAINERS | 1 + > drivers/iio/adc/ad7380.c | 143 +++++++++++++++++++-- > 3 files changed, 174 insertions(+), 8 deletions(-) > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-ad7380 b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad7380 > new file mode 100644 > index 000000000000..0a560ef3e32a > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad7380 > @@ -0,0 +1,38 @@ > +What: /sys/bus/iio/devices/iio:deviceX/oversampling_mode > +KernelVersion: 6.9 > +Contact: Michael Hennerich <michael.hennerich@analog.com> > +Description: > + Writing this attribute sets the oversampling average mode. > + Reading it, shows the configured mode. > + Available modes can be displayed using the oversampling_mode_available > + attribute. > + When writing this attribute to change the oversampling mode, this will > + have the following side effects: Where possible, write ABI docs with the assumption we will generalize them in future. Annoyingly the documentation system doesn't allow for multiple descriptions. As such, additional information like this doesn't belong in the ABI docs. > + > + - soft reset the ADC to flush the oversampling block and FIFO I think this was already picked up on in another review, but my inclination is make this something you can't change with the buffer enabled. The results will be rather unpredictable anyway and it will simplify the handling a little to just block that corner with a claim (or failure to claim) direct mode when setting this. > + > + - the available oversampling ratios depend on the oversampling mode > + configured so to avoid misconfiguration, changing the mode will disable > + the oversampling by setting the ratio to 1. Better to get a close as possible. If they've configured it to something we can't do then it's user error. If they have picked a value that is still possible then allowing that is a nice usability improvement. > + > + - the list of available ratios (displayed by reading the > + oversampling_ratio_available attribute) will be updated when changing > + the oversampling mode. In general an ABI element is allowed to modify any other (because this sort of chaining is common.) As such I don't think this needs to be in the ABI docs but would be a useful detail to add to a chip specific main document elsewhere. > + > +What: /sys/bus/iio/devices/iio:deviceX/oversampling_mode_available > +KernelVersion: 6.9 > +Contact: Michael Hennerich <michael.hennerich@analog.com> > +Description: > + Display the available oversampling average modes. The two available modes > + are "normal" and "rolling" where "normal" average mode is the default one. > + > + - normal averaging involves taking a number of samples, adding them > + together, and dividing the result by the number of samples taken. > + This result is then output from the device. The sample data is cleared > + when the process completes. Because we need more samples to output a > + value, the data output rate decrease with the oversampling ratio. > + > + - rolling oversampling mode uses a first in, first out (FIFO) buffer of > + the most recent samples in the averaging calculation, allowing the ADC > + throughput rate and output data rate to stay the same, since we only need > + to take only one sample for each new conversion. If we keep this, I wonder if "moving" or "rolling" is the more common term for this. > + > +static IIO_DEVICE_ATTR_RW(oversampling_mode, 0); > +static IIO_DEVICE_ATTR_RO(oversampling_mode_available, 0); > + > +static struct attribute *ad7380_attributes[] = { > + &iio_dev_attr_oversampling_mode.dev_attr.attr, > + &iio_dev_attr_oversampling_mode_available.dev_attr.attr, > + NULL > +}; > + > +static const struct attribute_group ad7380_attribute_group = { > + .attrs = ad7380_attributes, > +}; Bring the sysfs includes in here rather than in the original driver patch. Thanks, Jonathan
On Mon, May 6, 2024 at 9:17 AM Jonathan Cameron <jic23@kernel.org> wrote: > > On Wed, 01 May 2024 16:55:42 +0200 > Julien Stephan <jstephan@baylibre.com> wrote: > > > Adds support for rolling average oversampling mode. > > > > Rolling oversampling mode uses a first in, first out (FIFO) buffer of > > the most recent samples in the averaging calculation, allowing the ADC > > throughput rate and output data rate to stay the same, since we only need > > to take only one sample for each new conversion. > > > > The FIFO length is 8, thus the available oversampling ratios are 1, 2, 4, 8 > > in this mode (vs 1, 2, 4, 8, 16, 32 for the normal average) > > Ah. I should have read on! > > > > > In order to be able to change the averaging mode, this commit also adds > > the new "oversampling_mode" and "oversampling_mode_available" custom > > attributes along with the according documentation file in > > Documentation/ABI/testing/sysfs-bus-iio-adc-ad7380 since no standard > > attributes correspond to this use case. > > This comes to the comment I stuck in the previous patch. > > To most people this is not a form of oversampling because the data rate > remains unchanged. It's a cheap low pass filter (boxcar) Google pointed me at: > https://dsp.stackexchange.com/questions/9966/what-is-the-cut-off-frequency-of-a-moving-average-filter > > in_voltage_low_pass_3db_frequency would be the most appropriate standard > ABI for this if we do treat it as a low pass filter control. > > I'm not necessarily saying we don't want new ABI for this, but I would > like to consider the pros and cons of just using the 3db frequency. > > So would that work for this part or am I missing something? > I like the idea. But from the link, it looks like the 3dB frequency depends on the sampling frequency which is unknown (e.g. could come from hrtimer trigger). Would it be reasonable to calculate the 3db frequency at the max sample rate that the chip allows and just use those numbers?
On Mon, 6 May 2024 10:04:10 -0500 David Lechner <dlechner@baylibre.com> wrote: > On Mon, May 6, 2024 at 9:17 AM Jonathan Cameron <jic23@kernel.org> wrote: > > > > On Wed, 01 May 2024 16:55:42 +0200 > > Julien Stephan <jstephan@baylibre.com> wrote: > > > > > Adds support for rolling average oversampling mode. > > > > > > Rolling oversampling mode uses a first in, first out (FIFO) buffer of > > > the most recent samples in the averaging calculation, allowing the ADC > > > throughput rate and output data rate to stay the same, since we only need > > > to take only one sample for each new conversion. > > > > > > The FIFO length is 8, thus the available oversampling ratios are 1, 2, 4, 8 > > > in this mode (vs 1, 2, 4, 8, 16, 32 for the normal average) > > > > Ah. I should have read on! > > > > > > > > In order to be able to change the averaging mode, this commit also adds > > > the new "oversampling_mode" and "oversampling_mode_available" custom > > > attributes along with the according documentation file in > > > Documentation/ABI/testing/sysfs-bus-iio-adc-ad7380 since no standard > > > attributes correspond to this use case. > > > > This comes to the comment I stuck in the previous patch. > > > > To most people this is not a form of oversampling because the data rate > > remains unchanged. It's a cheap low pass filter (boxcar) Google pointed me at: > > https://dsp.stackexchange.com/questions/9966/what-is-the-cut-off-frequency-of-a-moving-average-filter > > > > in_voltage_low_pass_3db_frequency would be the most appropriate standard > > ABI for this if we do treat it as a low pass filter control. > > > > I'm not necessarily saying we don't want new ABI for this, but I would > > like to consider the pros and cons of just using the 3db frequency. > > > > So would that work for this part or am I missing something? > > > > I like the idea. But from the link, it looks like the 3dB frequency > depends on the sampling frequency which is unknown (e.g. could come > from hrtimer trigger). > > Would it be reasonable to calculate the 3db frequency at the max > sample rate that the chip allows and just use those numbers? > Ah. So looking at datasheet the normal average oversampling is self clocked, but this version is not. So, I'll ask the dumb question. What is this feature for? We have to pump the SPI bus anyway why not just do the maths in userspace? Oversampling is normally about data rate reduction with a bonus in precision obtained. Jonathan
On Wed, May 8, 2024 at 6:26 AM Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote: > > On Mon, 6 May 2024 10:04:10 -0500 > David Lechner <dlechner@baylibre.com> wrote: > > > On Mon, May 6, 2024 at 9:17 AM Jonathan Cameron <jic23@kernel.org> wrote: > > > > > > On Wed, 01 May 2024 16:55:42 +0200 > > > Julien Stephan <jstephan@baylibre.com> wrote: > > > > > > > Adds support for rolling average oversampling mode. > > > > > > > > Rolling oversampling mode uses a first in, first out (FIFO) buffer of > > > > the most recent samples in the averaging calculation, allowing the ADC > > > > throughput rate and output data rate to stay the same, since we only need > > > > to take only one sample for each new conversion. > > > > > > > > The FIFO length is 8, thus the available oversampling ratios are 1, 2, 4, 8 > > > > in this mode (vs 1, 2, 4, 8, 16, 32 for the normal average) > > > > > > Ah. I should have read on! > > > > > > > > > > > In order to be able to change the averaging mode, this commit also adds > > > > the new "oversampling_mode" and "oversampling_mode_available" custom > > > > attributes along with the according documentation file in > > > > Documentation/ABI/testing/sysfs-bus-iio-adc-ad7380 since no standard > > > > attributes correspond to this use case. > > > > > > This comes to the comment I stuck in the previous patch. > > > > > > To most people this is not a form of oversampling because the data rate > > > remains unchanged. It's a cheap low pass filter (boxcar) Google pointed me at: > > > https://dsp.stackexchange.com/questions/9966/what-is-the-cut-off-frequency-of-a-moving-average-filter > > > > > > in_voltage_low_pass_3db_frequency would be the most appropriate standard > > > ABI for this if we do treat it as a low pass filter control. > > > > > > I'm not necessarily saying we don't want new ABI for this, but I would > > > like to consider the pros and cons of just using the 3db frequency. > > > > > > So would that work for this part or am I missing something? > > > > > > > I like the idea. But from the link, it looks like the 3dB frequency > > depends on the sampling frequency which is unknown (e.g. could come > > from hrtimer trigger). > > > > Would it be reasonable to calculate the 3db frequency at the max > > sample rate that the chip allows and just use those numbers? > > > Ah. So looking at datasheet the normal average oversampling is > self clocked, but this version is not. > > So, I'll ask the dumb question. What is this feature for? > We have to pump the SPI bus anyway why not just do the maths in > userspace? Oversampling is normally about data rate reduction > with a bonus in precision obtained. > > Jonathan > I asked the apps engineers and the answer I got is that it a way to enable oversampling while still maintaining a high sample rate. Another thing to consider here is that we can only enable the extra resolution bits if oversampling is enabled (normal or rolling mode). The chip might not work right if we try to enable the extra bits without oversampling enabled. So my thinking is perhaps it is better to keep the rolling mode as oversampling rather than trying to call it a low pass filter. As you said, normal mode is about data rate reduction with bonus precision. Rolling average oversampling mode then would then just be for the bonus precision.
On Thu, 9 May 2024 17:01:14 -0500 David Lechner <dlechner@baylibre.com> wrote: > On Wed, May 8, 2024 at 6:26 AM Jonathan Cameron > <Jonathan.Cameron@huawei.com> wrote: > > > > On Mon, 6 May 2024 10:04:10 -0500 > > David Lechner <dlechner@baylibre.com> wrote: > > > > > On Mon, May 6, 2024 at 9:17 AM Jonathan Cameron <jic23@kernel.org> wrote: > > > > > > > > On Wed, 01 May 2024 16:55:42 +0200 > > > > Julien Stephan <jstephan@baylibre.com> wrote: > > > > > > > > > Adds support for rolling average oversampling mode. > > > > > > > > > > Rolling oversampling mode uses a first in, first out (FIFO) buffer of > > > > > the most recent samples in the averaging calculation, allowing the ADC > > > > > throughput rate and output data rate to stay the same, since we only need > > > > > to take only one sample for each new conversion. > > > > > > > > > > The FIFO length is 8, thus the available oversampling ratios are 1, 2, 4, 8 > > > > > in this mode (vs 1, 2, 4, 8, 16, 32 for the normal average) > > > > > > > > Ah. I should have read on! > > > > > > > > > > > > > > In order to be able to change the averaging mode, this commit also adds > > > > > the new "oversampling_mode" and "oversampling_mode_available" custom > > > > > attributes along with the according documentation file in > > > > > Documentation/ABI/testing/sysfs-bus-iio-adc-ad7380 since no standard > > > > > attributes correspond to this use case. > > > > > > > > This comes to the comment I stuck in the previous patch. > > > > > > > > To most people this is not a form of oversampling because the data rate > > > > remains unchanged. It's a cheap low pass filter (boxcar) Google pointed me at: > > > > https://dsp.stackexchange.com/questions/9966/what-is-the-cut-off-frequency-of-a-moving-average-filter > > > > > > > > in_voltage_low_pass_3db_frequency would be the most appropriate standard > > > > ABI for this if we do treat it as a low pass filter control. > > > > > > > > I'm not necessarily saying we don't want new ABI for this, but I would > > > > like to consider the pros and cons of just using the 3db frequency. > > > > > > > > So would that work for this part or am I missing something? > > > > > > > > > > I like the idea. But from the link, it looks like the 3dB frequency > > > depends on the sampling frequency which is unknown (e.g. could come > > > from hrtimer trigger). > > > > > > Would it be reasonable to calculate the 3db frequency at the max > > > sample rate that the chip allows and just use those numbers? > > > > > Ah. So looking at datasheet the normal average oversampling is > > self clocked, but this version is not. > > > > So, I'll ask the dumb question. What is this feature for? > > We have to pump the SPI bus anyway why not just do the maths in > > userspace? Oversampling is normally about data rate reduction > > with a bonus in precision obtained. > > > > Jonathan > > > > I asked the apps engineers and the answer I got is that it a way to > enable oversampling while still maintaining a high sample rate. If I read it correctly (and based on how this is done on some other devices) it's exactly the same as not enabling this mode and in software adding up the last 8 readings (so software oversampling). Data rate is the same and arguably the larger spi transfers that might result from their solution are more expensive than doing the sum on the CPU. A long time back there was IIRC some discussion of implementing this sort of filtering as a pluggable component between an IIO device driver and the kfifo - idea being to reduce data being passed to userspace - there would be no point in doing this variant though as the data rate isn't reduced. It would be easy to do as a buffer consumer IIO device that just does maths on incoming data before providing it's own kfifo, but no one ever implemented it. > > Another thing to consider here is that we can only enable the extra > resolution bits if oversampling is enabled (normal or rolling mode). > The chip might not work right if we try to enable the extra bits > without oversampling enabled. I think the key question is where do those extra bits come from? If this is conventional oversampling then we just sum up the readings and divide by how many there are. (A1 + A2 + A3 + A4 + A5 + A6 + A7) / 8 So sum plus shift right 3. However you don't actually do the div 8 you just change the scale to reflect that your new LSB is 1/8th of that before giving greater precision. This is giving 2 extra bits, so it's slightly worse than the 3 we'd get doing it in software (trade off to keep the SPI transfers smaller). So they are either dropping the LSB or just possibly dithering it. So far I'm not seeing a strong reason to support this functionality. > > So my thinking is perhaps it is better to keep the rolling mode as > oversampling rather than trying to call it a low pass filter. As you > said, normal mode is about data rate reduction with bonus precision. > Rolling average oversampling mode then would then just be for the > bonus precision. True if there actually is any. I took another close read through the datasheet sections and the description aligns with my assumptions above. Jonathan
diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-ad7380 b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad7380 new file mode 100644 index 000000000000..0a560ef3e32a --- /dev/null +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad7380 @@ -0,0 +1,38 @@ +What: /sys/bus/iio/devices/iio:deviceX/oversampling_mode +KernelVersion: 6.9 +Contact: Michael Hennerich <michael.hennerich@analog.com> +Description: + Writing this attribute sets the oversampling average mode. + Reading it, shows the configured mode. + Available modes can be displayed using the oversampling_mode_available + attribute. + When writing this attribute to change the oversampling mode, this will + have the following side effects: + + - soft reset the ADC to flush the oversampling block and FIFO + + - the available oversampling ratios depend on the oversampling mode + configured so to avoid misconfiguration, changing the mode will disable + the oversampling by setting the ratio to 1. + + - the list of available ratios (displayed by reading the + oversampling_ratio_available attribute) will be updated when changing + the oversampling mode. + +What: /sys/bus/iio/devices/iio:deviceX/oversampling_mode_available +KernelVersion: 6.9 +Contact: Michael Hennerich <michael.hennerich@analog.com> +Description: + Display the available oversampling average modes. The two available modes + are "normal" and "rolling" where "normal" average mode is the default one. + + - normal averaging involves taking a number of samples, adding them + together, and dividing the result by the number of samples taken. + This result is then output from the device. The sample data is cleared + when the process completes. Because we need more samples to output a + value, the data output rate decrease with the oversampling ratio. + + - rolling oversampling mode uses a first in, first out (FIFO) buffer of + the most recent samples in the averaging calculation, allowing the ADC + throughput rate and output data rate to stay the same, since we only need + to take only one sample for each new conversion. diff --git a/MAINTAINERS b/MAINTAINERS index 87724a9e9f9f..ca1e115f2aff 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -434,6 +434,7 @@ R: David Lechner <dlechner@baylibre.com> S: Supported W: https://wiki.analog.com/resources/tools-software/linux-drivers/iio-adc/ad738x W: https://ez.analog.com/linux-software-drivers +F: Documentation/ABI/testing/sysfs-bus-iio-adc-ad7380 F: Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml F: drivers/iio/adc/ad7380.c diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c index 1e3869f5e48c..7b021bb9cf87 100644 --- a/drivers/iio/adc/ad7380.c +++ b/drivers/iio/adc/ad7380.c @@ -51,6 +51,8 @@ #define AD7380_REG_ADDR_ALERT_HIGH_TH 0x5 #define AD7380_CONFIG1_OS_MODE BIT(9) +#define OS_MODE_NORMAL_AVERAGE 0 +#define OS_MODE_ROLLING_AVERAGE 1 #define AD7380_CONFIG1_OSR GENMASK(8, 6) #define AD7380_CONFIG1_CRC_W BIT(5) #define AD7380_CONFIG1_CRC_R BIT(4) @@ -159,16 +161,27 @@ static const struct ad7380_timing_specs ad7380_4_timing = { .t_csh_ns = 20, }; +/* + * Available oversampling modes. + */ +static const char * const ad7380_oversampling_average_modes[] = { + [OS_MODE_NORMAL_AVERAGE] = "normal", + [OS_MODE_ROLLING_AVERAGE] = "rolling", +}; + /* * Available oversampling ratios. The indices correspond * with the bit value expected by the chip. - * The available ratios depend on the averaging mode, - * only normal averaging is supported for now + * The available ratios depend on the averaging mode. */ static const int ad7380_normal_average_oversampling_ratios[] = { 1, 2, 4, 8, 16, 32, }; +static const int ad7380_rolling_average_oversampling_ratios[] = { + 1, 2, 4, 8, +}; + static const struct ad7380_chip_info ad7380_chip_info = { .name = "ad7380", .channels = ad7380_channels, @@ -244,6 +257,7 @@ static const struct ad7380_chip_info ad7384_4_chip_info = { struct ad7380_state { const struct ad7380_chip_info *chip_info; struct spi_device *spi; + unsigned int oversampling_mode; unsigned int oversampling_ratio; struct regmap *regmap; unsigned int vref_mv; @@ -403,7 +417,7 @@ static int ad7380_read_direct(struct ad7380_state *st, /* * In normal average oversampling we need to wait for multiple conversions to be done */ - if (st->oversampling_ratio > 1) + if (st->oversampling_mode == OS_MODE_NORMAL_AVERAGE && st->oversampling_ratio > 1) xfers[0].delay.value = T_CONVERT_NS + 500 * st->oversampling_ratio; ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers)); @@ -462,10 +476,22 @@ static int ad7380_read_avail(struct iio_dev *indio_dev, const int **vals, int *type, int *length, long mask) { + struct ad7380_state *st = iio_priv(indio_dev); + switch (mask) { case IIO_CHAN_INFO_OVERSAMPLING_RATIO: - *vals = ad7380_normal_average_oversampling_ratios; - *length = ARRAY_SIZE(ad7380_normal_average_oversampling_ratios); + switch (st->oversampling_mode) { + case OS_MODE_NORMAL_AVERAGE: + *vals = ad7380_normal_average_oversampling_ratios; + *length = ARRAY_SIZE(ad7380_normal_average_oversampling_ratios); + break; + case OS_MODE_ROLLING_AVERAGE: + *vals = ad7380_rolling_average_oversampling_ratios; + *length = ARRAY_SIZE(ad7380_rolling_average_oversampling_ratios); + break; + default: + return -EINVAL; + } *type = IIO_VAL_INT; return IIO_AVAIL_LIST; @@ -505,9 +531,20 @@ static int ad7380_write_raw(struct iio_dev *indio_dev, switch (mask) { case IIO_CHAN_INFO_OVERSAMPLING_RATIO: - osr = check_osr(ad7380_normal_average_oversampling_ratios, - ARRAY_SIZE(ad7380_normal_average_oversampling_ratios), - val); + switch (st->oversampling_mode) { + case OS_MODE_NORMAL_AVERAGE: + osr = check_osr(ad7380_normal_average_oversampling_ratios, + ARRAY_SIZE(ad7380_normal_average_oversampling_ratios), + val); + break; + case OS_MODE_ROLLING_AVERAGE: + osr = check_osr(ad7380_rolling_average_oversampling_ratios, + ARRAY_SIZE(ad7380_rolling_average_oversampling_ratios), + val); + break; + default: + return -EINVAL; + } if (osr < 0) return osr; @@ -538,7 +575,96 @@ static int ad7380_write_raw(struct iio_dev *indio_dev, } } +static ssize_t oversampling_mode_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct ad7380_state *st = iio_priv(dev_to_iio_dev(dev)); + unsigned int os_mode; + + os_mode = st->oversampling_mode; + + return sysfs_emit(buf, "%s\n", ad7380_oversampling_average_modes[os_mode]); +} + +static ssize_t oversampling_mode_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t len) +{ + struct iio_dev *indio_dev = dev_to_iio_dev(dev); + struct ad7380_state *st = iio_priv(indio_dev); + int os_mode, ret; + + ret = sysfs_match_string(ad7380_oversampling_average_modes, buf); + if (ret < 0) + return ret; + + os_mode = ret; + + iio_device_claim_direct_scoped(return -EBUSY, indio_dev) { + ret = regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG1, + AD7380_CONFIG1_OS_MODE, + FIELD_PREP(AD7380_CONFIG1_OS_MODE, os_mode)); + + if (ret) + return ret; + + st->oversampling_mode = os_mode; + + /* + * Oversampling ratio depends on oversampling mode, to avoid + * misconfiguration when changing oversampling mode, + * disable oversampling by setting OSR to 0. + */ + ret = regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG1, + AD7380_CONFIG1_OSR, FIELD_PREP(AD7380_CONFIG1_OSR, 0)); + + if (ret) + return ret; + + st->oversampling_ratio = 1; + + /* + * Perform a soft reset. + * This will flush the oversampling block and FIFO but will + * maintain the content of the configurable registers. + */ + ret = regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG2, + AD7380_CONFIG2_RESET, + FIELD_PREP(AD7380_CONFIG2_RESET, + AD7380_CONFIG2_RESET_SOFT)); + } + return ret ?: len; +} + +static ssize_t oversampling_mode_available_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + int i; + size_t len = 0; + + for (i = 0; i < ARRAY_SIZE(ad7380_oversampling_average_modes); i++) + len += sysfs_emit_at(buf, len, "%s ", ad7380_oversampling_average_modes[i]); + + buf[len - 1] = '\n'; + + return len; +} + +static IIO_DEVICE_ATTR_RW(oversampling_mode, 0); +static IIO_DEVICE_ATTR_RO(oversampling_mode_available, 0); + +static struct attribute *ad7380_attributes[] = { + &iio_dev_attr_oversampling_mode.dev_attr.attr, + &iio_dev_attr_oversampling_mode_available.dev_attr.attr, + NULL +}; + +static const struct attribute_group ad7380_attribute_group = { + .attrs = ad7380_attributes, +}; + static const struct iio_info ad7380_info = { + .attrs = &ad7380_attribute_group, .read_raw = &ad7380_read_raw, .read_avail = &ad7380_read_avail, .write_raw = &ad7380_write_raw, @@ -569,6 +695,7 @@ static int ad7380_init(struct ad7380_state *st, struct regulator *vref) * This is the default value after reset, * so just initialize internal data */ + st->oversampling_mode = OS_MODE_NORMAL_AVERAGE; st->oversampling_ratio = 1; /* SPI 1-wire mode */
Adds support for rolling average oversampling mode. Rolling oversampling mode uses a first in, first out (FIFO) buffer of the most recent samples in the averaging calculation, allowing the ADC throughput rate and output data rate to stay the same, since we only need to take only one sample for each new conversion. The FIFO length is 8, thus the available oversampling ratios are 1, 2, 4, 8 in this mode (vs 1, 2, 4, 8, 16, 32 for the normal average) In order to be able to change the averaging mode, this commit also adds the new "oversampling_mode" and "oversampling_mode_available" custom attributes along with the according documentation file in Documentation/ABI/testing/sysfs-bus-iio-adc-ad7380 since no standard attributes correspond to this use case. Signed-off-by: Julien Stephan <jstephan@baylibre.com> --- Documentation/ABI/testing/sysfs-bus-iio-adc-ad7380 | 38 ++++++ MAINTAINERS | 1 + drivers/iio/adc/ad7380.c | 143 +++++++++++++++++++-- 3 files changed, 174 insertions(+), 8 deletions(-)