* [PATCH v3 0/4] Add support for ADAQ776x-1 ADC Family
@ 2025-09-05 9:48 Jonathan Santos
2025-09-05 9:49 ` [PATCH v3 1/4] dt-bindings: iio: adc: ad7768-1: add new supported parts Jonathan Santos
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Jonathan Santos @ 2025-09-05 9:48 UTC (permalink / raw)
To: linux-iio, devicetree, linux-kernel
Cc: Jonathan Santos, lars, jic23, dlechner, nuno.sa, andy, robh,
krzk+dt, conor+dt, marcelo.schmitt1, jonath4nns
This adds support for the ADAQ7767-1, ADAQ7768-1 and ADAQ7769-1 devices.
The ADAQ7768-1 and ADAQ7769-1 integrate a programmable gain amplifier (PGA)
with 7 and 8 gain options, respectively. The ADAQ7767-1 and ADAQ7769-1
also feature a 3-pin selectable Anti-aliasing filter (AAF) gain.
---
Changes in v3:
* Renamed adi,gain-milli to adi,aaf-gain-bp. Now it represents basis points
(one hundredth of a percent).
* ad7768_channel_masks removed along with available_masks element in
ad7768_chip_info struct. It does not add anything for single channels,
so not needed, at least for now.
* New patch adding 64-bit fractional number types to math.h.
* Moved aaf gain parsing to its own function, and now returning after
warning to avoid setting a variable when it shouldn't (avoid confusion).
* ad7768_set_pga_gain(): removed the pgia enable check, relying on the
regmap cache.
* Addressed other review comments, see individual patches.
Changes in v2:
* adi,aaf-gain property renamed to adi,gain-milli. Default value added.
* fixed some commit messages.
* Added 'select RATIONAL' to Kconfig.
* Added lock to protect PGA value access.
* rewrote AAF gain check and replaced error returns with warnings.
* Addressed other review comments, see individual patches.
* Link to v1: https://lore.kernel.org/linux-iio/cover.1754617360.git.Jonathan.Santos@analog.com/T/#t
---
Jonathan Santos (4):
dt-bindings: iio: adc: ad7768-1: add new supported parts
iio: adc: ad7768-1: introduce chip info for future multidevice support
math.h: Add 64-bit fractional numbers types
iio: adc: ad7768-1: add support for ADAQ776x-1 ADC Family
.../bindings/iio/adc/adi,ad7768-1.yaml | 44 ++-
drivers/iio/adc/Kconfig | 1 +
drivers/iio/adc/ad7768-1.c | 359 ++++++++++++++++--
include/linux/math.h | 2 +
4 files changed, 377 insertions(+), 29 deletions(-)
base-commit: 91812d3843409c235f336f32f1c37ddc790f1e03
--
2.34.1
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v3 1/4] dt-bindings: iio: adc: ad7768-1: add new supported parts 2025-09-05 9:48 [PATCH v3 0/4] Add support for ADAQ776x-1 ADC Family Jonathan Santos @ 2025-09-05 9:49 ` Jonathan Santos 2025-09-05 17:06 ` David Lechner 2025-09-05 9:49 ` [PATCH v3 2/4] iio: adc: ad7768-1: introduce chip info for future multidevice support Jonathan Santos ` (2 subsequent siblings) 3 siblings, 1 reply; 13+ messages in thread From: Jonathan Santos @ 2025-09-05 9:49 UTC (permalink / raw) To: linux-iio, devicetree, linux-kernel Cc: Jonathan Santos, lars, jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, marcelo.schmitt1, jonath4nns Add compatibles for supported parts in the ad7768-1 family: ADAQ7767-1, ADAQ7768-1 and ADAQ7769-1 Add property and checks for AFF gain, supported by ADAQ7767-1 and ADAQ7769-1 parts: adi,aaf-gain-bp Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com> --- v3 Changes: * Renamed adi,gain-milli to adi,aaf-gain-bp. Now it represents basis points (one hundredth of a percent) as suggested by Krzysztof. Description was adjusted. Note: permille (1/1000) was also suggested as unit for this property. v2 Changes: * adi,aaf-gain property renamed to adi,gain-milli. Description was simplified. * default value add to adi,gain-milli. --- .../bindings/iio/adc/adi,ad7768-1.yaml | 44 +++++++++++++++++-- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml index c06d0fc791d3..c2ad8e585586 100644 --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml @@ -4,18 +4,26 @@ $id: http://devicetree.org/schemas/iio/adc/adi,ad7768-1.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml# -title: Analog Devices AD7768-1 ADC device driver +title: Analog Devices AD7768-1 ADC family maintainers: - Michael Hennerich <michael.hennerich@analog.com> description: | - Datasheet at: - https://www.analog.com/media/en/technical-documentation/data-sheets/ad7768-1.pdf + Analog Devices AD7768-1 24-Bit Single Channel Low Power sigma-delta ADC family + + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7768-1.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/adaq7767-1.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/adaq7768-1.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/adaq7769-1.pdf properties: compatible: - const: adi,ad7768-1 + enum: + - adi,ad7768-1 + - adi,adaq7767-1 + - adi,adaq7768-1 + - adi,adaq7769-1 reg: maxItems: 1 @@ -58,6 +66,19 @@ properties: description: ADC reference voltage supply + adi,aaf-gain-bp: + description: | + Specifies the gain applied by the Analog Anti-Aliasing Filter (AAF) + to the ADC input in basis points (one hundredth of a percent). + The hardware gain is determined by which input pin(s) the signal goes + through into the AAF. The possible connections are: + * For the ADAQ7767-1: Input connected to IN1±, IN2± or IN3±. + * For the ADAQ7769-1: OUT_PGA pin connected to IN1_AAF+, IN2_AAF+, + or IN3_AAF+. + $ref: /schemas/types.yaml#/definitions/uint16 + enum: [1430, 3640, 10000] + default: 10000 + adi,sync-in-gpios: maxItems: 1 description: @@ -147,6 +168,21 @@ patternProperties: allOf: - $ref: /schemas/spi/spi-peripheral-props.yaml# + # AAF Gain property only applies to ADAQ7767-1 and ADAQ7769-1 devices + - if: + properties: + compatible: + contains: + enum: + - adi,adaq7767-1 + - adi,adaq7769-1 + then: + required: + - adi,aaf-gain-bp + else: + properties: + adi,aaf-gain-bp: false + unevaluatedProperties: false examples: -- 2.34.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/4] dt-bindings: iio: adc: ad7768-1: add new supported parts 2025-09-05 9:49 ` [PATCH v3 1/4] dt-bindings: iio: adc: ad7768-1: add new supported parts Jonathan Santos @ 2025-09-05 17:06 ` David Lechner 0 siblings, 0 replies; 13+ messages in thread From: David Lechner @ 2025-09-05 17:06 UTC (permalink / raw) To: Jonathan Santos, linux-iio, devicetree, linux-kernel Cc: lars, jic23, nuno.sa, andy, robh, krzk+dt, conor+dt, marcelo.schmitt1, jonath4nns On 9/5/25 4:49 AM, Jonathan Santos wrote: > Add compatibles for supported parts in the ad7768-1 family: > ADAQ7767-1, ADAQ7768-1 and ADAQ7769-1 > > Add property and checks for AFF gain, supported by ADAQ7767-1 > and ADAQ7769-1 parts: > adi,aaf-gain-bp > > Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com> > --- > v3 Changes: > * Renamed adi,gain-milli to adi,aaf-gain-bp. Now it represents basis points > (one hundredth of a percent) as suggested by Krzysztof. Description was > adjusted. > Note: permille (1/1000) was also suggested as unit for this property. > > v2 Changes: > * adi,aaf-gain property renamed to adi,gain-milli. Description was > simplified. > * default value add to adi,gain-milli. > --- > .../bindings/iio/adc/adi,ad7768-1.yaml | 44 +++++++++++++++++-- > 1 file changed, 40 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml > index c06d0fc791d3..c2ad8e585586 100644 > --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml > @@ -4,18 +4,26 @@ > $id: http://devicetree.org/schemas/iio/adc/adi,ad7768-1.yaml# > $schema: http://devicetree.org/meta-schemas/core.yaml# > > -title: Analog Devices AD7768-1 ADC device driver > +title: Analog Devices AD7768-1 ADC family > > maintainers: > - Michael Hennerich <michael.hennerich@analog.com> > > description: | > - Datasheet at: > - https://www.analog.com/media/en/technical-documentation/data-sheets/ad7768-1.pdf > + Analog Devices AD7768-1 24-Bit Single Channel Low Power sigma-delta ADC family > + > + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7768-1.pdf > + https://www.analog.com/media/en/technical-documentation/data-sheets/adaq7767-1.pdf > + https://www.analog.com/media/en/technical-documentation/data-sheets/adaq7768-1.pdf > + https://www.analog.com/media/en/technical-documentation/data-sheets/adaq7769-1.pdf Only adding one property seems a bit incomplete give that the ADAQs have significantly more pins that the regular ADC. E.g different/more power supplies; gain-gpios, fda-gpios (or don't allow gpio-controller if we can assume GAINx and FAx are always wired to back to the ADC and not coming from somewhere else). > > properties: > compatible: > - const: adi,ad7768-1 > + enum: > + - adi,ad7768-1 > + - adi,adaq7767-1 > + - adi,adaq7768-1 > + - adi,adaq7769-1 > > reg: > maxItems: 1 > @@ -58,6 +66,19 @@ properties: > description: > ADC reference voltage supply > > + adi,aaf-gain-bp: > + description: | > + Specifies the gain applied by the Analog Anti-Aliasing Filter (AAF) > + to the ADC input in basis points (one hundredth of a percent). > + The hardware gain is determined by which input pin(s) the signal goes > + through into the AAF. The possible connections are: > + * For the ADAQ7767-1: Input connected to IN1±, IN2± or IN3±. > + * For the ADAQ7769-1: OUT_PGA pin connected to IN1_AAF+, IN2_AAF+, > + or IN3_AAF+. > + $ref: /schemas/types.yaml#/definitions/uint16 > + enum: [1430, 3640, 10000] > + default: 10000 > + ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 2/4] iio: adc: ad7768-1: introduce chip info for future multidevice support 2025-09-05 9:48 [PATCH v3 0/4] Add support for ADAQ776x-1 ADC Family Jonathan Santos 2025-09-05 9:49 ` [PATCH v3 1/4] dt-bindings: iio: adc: ad7768-1: add new supported parts Jonathan Santos @ 2025-09-05 9:49 ` Jonathan Santos 2025-09-05 16:51 ` Andy Shevchenko 2025-09-05 17:06 ` David Lechner 2025-09-05 9:49 ` [PATCH v3 3/4] math.h: Add 64-bit fractional numbers types Jonathan Santos 2025-09-05 9:49 ` [PATCH v3 4/4] iio: adc: ad7768-1: add support for ADAQ776x-1 ADC Family Jonathan Santos 3 siblings, 2 replies; 13+ messages in thread From: Jonathan Santos @ 2025-09-05 9:49 UTC (permalink / raw) To: linux-iio, devicetree, linux-kernel Cc: Jonathan Santos, lars, jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, marcelo.schmitt1, jonath4nns Add Chip info struct in SPI device to store channel information for each supported part. Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com> --- v3 Changes: * ad7768_channel_masks removed along with available_masks element in ad7768_chip_info struct. It does not add anything for single channels, so not needed, at least for now. * fixed inconsistency in spaces before \ in AD7768_CHAN macro. v2 Changes: * removed AD7768_CHAN_INFO_NONE macro. * reordered fields in ad7768_chip_info struct. * removed trailing comma. --- drivers/iio/adc/ad7768-1.c | 67 +++++++++++++++++++++++++------------- 1 1 file changed, 44 insertions(+), 23 deletions(-) diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c index 872c88d0c86c..000d294c616c 100644 --- a/drivers/iio/adc/ad7768-1.c +++ b/drivers/iio/adc/ad7768-1.c @@ -213,6 +213,12 @@ static const struct iio_scan_type ad7768_scan_type[] = { }, }; +struct ad7768_chip_info { + const char *name; + const struct iio_chan_spec *channel_spec; + int num_channels; +}; + struct ad7768_state { struct spi_device *spi; struct regmap *regmap; @@ -234,6 +240,7 @@ struct ad7768_state { struct gpio_desc *gpio_reset; const char *labels[AD7768_MAX_CHANNELS]; struct gpio_chip gpiochip; + const struct ad7768_chip_info *chip; bool en_spi_sync; /* * DMA (thus cache coherency maintenance) may require the @@ -748,24 +755,27 @@ static const struct iio_chan_spec_ext_info ad7768_ext_info[] = { { } }; +#define AD7768_CHAN(_idx, _msk_avail) { \ + .type = IIO_VOLTAGE, \ + .info_mask_separate_available = _msk_avail, \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \ + BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY) | \ + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \ + .info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \ + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \ + .info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ), \ + .ext_info = ad7768_ext_info, \ + .indexed = 1, \ + .channel = _idx, \ + .scan_index = _idx, \ + .has_ext_scan_type = 1, \ + .ext_scan_type = ad7768_scan_type, \ + .num_ext_scan_type = ARRAY_SIZE(ad7768_scan_type), \ +} + static const struct iio_chan_spec ad7768_channels[] = { - { - .type = IIO_VOLTAGE, - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | - BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY) | - BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), - .info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), - .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), - .info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ), - .ext_info = ad7768_ext_info, - .indexed = 1, - .channel = 0, - .scan_index = 0, - .has_ext_scan_type = 1, - .ext_scan_type = ad7768_scan_type, - .num_ext_scan_type = ARRAY_SIZE(ad7768_scan_type), - }, + AD7768_CHAN(0, 0), }; static int ad7768_read_raw(struct iio_dev *indio_dev, @@ -1321,6 +1331,12 @@ static int ad7768_register_regulators(struct device *dev, struct ad7768_state *s return 0; } +static const struct ad7768_chip_info ad7768_chip_info = { + .name = "ad7768-1", + .channel_spec = ad7768_channels, + .num_channels = ARRAY_SIZE(ad7768_channels), +}; + static int ad7768_probe(struct spi_device *spi) { struct ad7768_state *st; @@ -1371,9 +1387,14 @@ static int ad7768_probe(struct spi_device *spi) st->mclk_freq = clk_get_rate(st->mclk); - indio_dev->channels = ad7768_channels; - indio_dev->num_channels = ARRAY_SIZE(ad7768_channels); - indio_dev->name = spi_get_device_id(spi)->name; + st->chip = spi_get_device_match_data(spi); + if (!st->chip) + return dev_err_probe(&spi->dev, -ENODEV, + "Could not find chip info data\n"); + + indio_dev->channels = st->chip->channel_spec; + indio_dev->num_channels = st->chip->num_channels; + indio_dev->name = st->chip->name; indio_dev->info = &ad7768_info; indio_dev->modes = INDIO_DIRECT_MODE; @@ -1390,7 +1411,7 @@ static int ad7768_probe(struct spi_device *spi) init_completion(&st->completion); - ret = ad7768_set_channel_label(indio_dev, ARRAY_SIZE(ad7768_channels)); + ret = ad7768_set_channel_label(indio_dev, st->chip->num_channels); if (ret) return ret; @@ -1409,13 +1430,13 @@ static int ad7768_probe(struct spi_device *spi) } static const struct spi_device_id ad7768_id_table[] = { - { "ad7768-1", 0 }, + { "ad7768-1", (kernel_ulong_t)&ad7768_chip_info }, { } }; MODULE_DEVICE_TABLE(spi, ad7768_id_table); static const struct of_device_id ad7768_of_match[] = { - { .compatible = "adi,ad7768-1" }, + { .compatible = "adi,ad7768-1", .data = &ad7768_chip_info }, { } }; MODULE_DEVICE_TABLE(of, ad7768_of_match); -- 2.34.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/4] iio: adc: ad7768-1: introduce chip info for future multidevice support 2025-09-05 9:49 ` [PATCH v3 2/4] iio: adc: ad7768-1: introduce chip info for future multidevice support Jonathan Santos @ 2025-09-05 16:51 ` Andy Shevchenko 2025-09-05 17:06 ` David Lechner 1 sibling, 0 replies; 13+ messages in thread From: Andy Shevchenko @ 2025-09-05 16:51 UTC (permalink / raw) To: Jonathan Santos Cc: linux-iio, devicetree, linux-kernel, lars, jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, marcelo.schmitt1, jonath4nns On Fri, Sep 05, 2025 at 06:49:21AM -0300, Jonathan Santos wrote: > Add Chip info struct in SPI device to store channel information for > each supported part. ... > +#define AD7768_CHAN(_idx, _msk_avail) { \ I consider slightly better to read #define AD7768_CHAN(_idx, _msk_avail) \ { \ > + .type = IIO_VOLTAGE, \ > + .info_mask_separate_available = _msk_avail, \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \ > + BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY) | \ > + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \ In the original code below indentation was not broken. > + .info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \ > + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \ > + .info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ), \ > + .ext_info = ad7768_ext_info, \ > + .indexed = 1, \ > + .channel = _idx, \ > + .scan_index = _idx, \ > + .has_ext_scan_type = 1, \ > + .ext_scan_type = ad7768_scan_type, \ > + .num_ext_scan_type = ARRAY_SIZE(ad7768_scan_type), \ > +} ... > + st->chip = spi_get_device_match_data(spi); > + if (!st->chip) > + return dev_err_probe(&spi->dev, -ENODEV, > + "Could not find chip info data\n"); Might be useful in some cases, but I don't see its value. Just always provide a chip_info and no need to care about this. Esp. this just shows that it's mandatory, but if absent, it will crash on the following line. Since it's about probe, one will notice it immediately, otherwise it will mean a submission of the code that has never been tested. TL;DR: just drop this check. > + indio_dev->channels = st->chip->channel_spec; -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/4] iio: adc: ad7768-1: introduce chip info for future multidevice support 2025-09-05 9:49 ` [PATCH v3 2/4] iio: adc: ad7768-1: introduce chip info for future multidevice support Jonathan Santos 2025-09-05 16:51 ` Andy Shevchenko @ 2025-09-05 17:06 ` David Lechner 1 sibling, 0 replies; 13+ messages in thread From: David Lechner @ 2025-09-05 17:06 UTC (permalink / raw) To: Jonathan Santos, linux-iio, devicetree, linux-kernel Cc: lars, jic23, nuno.sa, andy, robh, krzk+dt, conor+dt, marcelo.schmitt1, jonath4nns On 9/5/25 4:49 AM, Jonathan Santos wrote: > Add Chip info struct in SPI device to store channel information for > each supported part. > ... > @@ -1371,9 +1387,14 @@ static int ad7768_probe(struct spi_device *spi) > > st->mclk_freq = clk_get_rate(st->mclk); > > - indio_dev->channels = ad7768_channels; > - indio_dev->num_channels = ARRAY_SIZE(ad7768_channels); > - indio_dev->name = spi_get_device_id(spi)->name; > + st->chip = spi_get_device_match_data(spi); Generally, we want this early in probe so that chip info is available as early as possible. Might not need it now, but would save us from having to move this later if we ever do. > + if (!st->chip) > + return dev_err_probe(&spi->dev, -ENODEV, > + "Could not find chip info data\n"); > + ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 3/4] math.h: Add 64-bit fractional numbers types 2025-09-05 9:48 [PATCH v3 0/4] Add support for ADAQ776x-1 ADC Family Jonathan Santos 2025-09-05 9:49 ` [PATCH v3 1/4] dt-bindings: iio: adc: ad7768-1: add new supported parts Jonathan Santos 2025-09-05 9:49 ` [PATCH v3 2/4] iio: adc: ad7768-1: introduce chip info for future multidevice support Jonathan Santos @ 2025-09-05 9:49 ` Jonathan Santos 2025-09-05 16:45 ` Andy Shevchenko 2025-09-05 9:49 ` [PATCH v3 4/4] iio: adc: ad7768-1: add support for ADAQ776x-1 ADC Family Jonathan Santos 3 siblings, 1 reply; 13+ messages in thread From: Jonathan Santos @ 2025-09-05 9:49 UTC (permalink / raw) To: linux-iio, devicetree, linux-kernel Cc: Jonathan Santos, lars, jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, marcelo.schmitt1, jonath4nns Extend fractional numbers types to include __u64 and __s64 data types. Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com> --- v3 Changes: * New patch. * OBS: Andy suggested to support long types, but the macro was made for data types like __TYPE. So i have added the __u64 and __s64 types. --- include/linux/math.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/linux/math.h b/include/linux/math.h index 0198c92cbe3e..ff28a0dcfaa8 100644 --- a/include/linux/math.h +++ b/include/linux/math.h @@ -130,6 +130,8 @@ __STRUCT_FRACT(s16) __STRUCT_FRACT(u16) __STRUCT_FRACT(s32) __STRUCT_FRACT(u32) +__STRUCT_FRACT(s64) +__STRUCT_FRACT(u64) #undef __STRUCT_FRACT /* Calculate "x * n / d" without unnecessary overflow or loss of precision. */ -- 2.34.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/4] math.h: Add 64-bit fractional numbers types 2025-09-05 9:49 ` [PATCH v3 3/4] math.h: Add 64-bit fractional numbers types Jonathan Santos @ 2025-09-05 16:45 ` Andy Shevchenko 0 siblings, 0 replies; 13+ messages in thread From: Andy Shevchenko @ 2025-09-05 16:45 UTC (permalink / raw) To: Jonathan Santos Cc: linux-iio, devicetree, linux-kernel, lars, jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, marcelo.schmitt1, jonath4nns On Fri, Sep 05, 2025 at 06:49:32AM -0300, Jonathan Santos wrote: > Extend fractional numbers types to include __u64 and __s64 data types. Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com> -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 4/4] iio: adc: ad7768-1: add support for ADAQ776x-1 ADC Family 2025-09-05 9:48 [PATCH v3 0/4] Add support for ADAQ776x-1 ADC Family Jonathan Santos ` (2 preceding siblings ...) 2025-09-05 9:49 ` [PATCH v3 3/4] math.h: Add 64-bit fractional numbers types Jonathan Santos @ 2025-09-05 9:49 ` Jonathan Santos 2025-09-05 17:08 ` David Lechner 2025-09-05 17:23 ` Andy Shevchenko 3 siblings, 2 replies; 13+ messages in thread From: Jonathan Santos @ 2025-09-05 9:49 UTC (permalink / raw) To: linux-iio, devicetree, linux-kernel Cc: Jonathan Santos, lars, jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, marcelo.schmitt1, jonath4nns Add support for ADAQ7767/68/69-1 series, which includes PGIA and Anti-aliasing filter (AAF) gains. Unlike the AD7768-1, they do not provide a VCM regulator interface. The PGA gain is configured in run-time through the scale attribute, if supported by the device. PGA is enabled and controlled by the GPIO controller (GPIOs 0, 1 and 2) provided by the device with the SPI interface. The AAF gain is defined by hardware connections and should be specified in the device tree. Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com> --- v3 Changes: * Fixed trailing comma issues. * Addressed other minor issues related to dead code, variable declaration, etc. * removed unnecessary comments and relocating some local variables. * replaced mutex_init() with devm_mutex_init(). * adopted different variables for the input and output of rational_best_approximation(). Also used a u64_fract for the inputs, but kept the unsigned long for the outputs, because could not create a unsigned long fraction number type. * ad7768_set_pga_gain(): removed the pgia enable check, relying on the regmap cache. * Moved aaf gain parsing to its own function, and now returning after warning to avoid setting a variable when it shouldn't (avoid confusion). * AAF gain is now in basis point units, so related multipliers and dividers are adjusted accordingly. v2 Changes: * Added more details to the commit message. * Some devices does not provide VCM regulator, so a new field in the chip info struct was added to indicate this. * Added 'select RATIONAL' to Kconfig. Kernel test robot pointed out compilation error due to undefined reference to rational_best_approximation(). * Added lock to protect PGA value access. * precision in the PGA calculation is now dependent of the channel sign (signed or unsigned). * went back to the original scale computation: (st->vref_uv * 2) / 2^n instead of st->vref_uv / 2^(n-1). * rewrote AAF gain check and replaced error returns with warnings. I the AAF gain is not provided, a default value is used. * Addressed other minor suggestions. --- drivers/iio/adc/Kconfig | 1 + drivers/iio/adc/ad7768-1.c | 292 ++++++++++++++++++++++++++++++++++++- 2 files changed, 291 insertions(+), 2 deletions(-) diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index 6de2abad0197..7c5fe3954514 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -374,6 +374,7 @@ config AD7768_1 depends on SPI select REGULATOR select REGMAP_SPI + select RATIONAL select IIO_BUFFER select IIO_TRIGGER select IIO_TRIGGERED_BUFFER diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c index 000d294c616c..c2951de4799d 100644 --- a/drivers/iio/adc/ad7768-1.c +++ b/drivers/iio/adc/ad7768-1.c @@ -6,6 +6,7 @@ */ #include <linux/array_size.h> #include <linux/bitfield.h> +#include <linux/cleanup.h> #include <linux/clk.h> #include <linux/completion.h> #include <linux/delay.h> @@ -14,8 +15,12 @@ #include <linux/gpio/driver.h> #include <linux/gpio/consumer.h> #include <linux/interrupt.h> +#include <linux/limits.h> +#include <linux/math.h> #include <linux/minmax.h> #include <linux/module.h> +#include <linux/mutex.h> +#include <linux/rational.h> #include <linux/regmap.h> #include <linux/regulator/consumer.h> #include <linux/regulator/driver.h> @@ -98,15 +103,21 @@ /* AD7768_REG_GPIO_CONTROL */ #define AD7768_GPIO_UNIVERSAL_EN BIT(7) #define AD7768_GPIO_CONTROL_MSK GENMASK(3, 0) +#define AD7768_GPIO_PGIA_EN (AD7768_GPIO_UNIVERSAL_EN | GENMASK(2, 0)) /* AD7768_REG_GPIO_WRITE */ #define AD7768_GPIO_WRITE_MSK GENMASK(3, 0) +#define AD7768_GPIO_WRITE(x) FIELD_PREP(AD7768_GPIO_WRITE_MSK, x) /* AD7768_REG_GPIO_READ */ #define AD7768_GPIO_READ_MSK GENMASK(3, 0) +#define AD7768_GPIO_READ(x) FIELD_PREP(AD7768_GPIO_READ_MSK, x) #define AD7768_VCM_OFF 0x07 +#define ADAQ776X_GAIN_MAX_NANO (128 * NANO) +#define ADAQ776X_MAX_GAIN_MODES 8 + #define AD7768_TRIGGER_SOURCE_SYNC_IDX 0 #define AD7768_MAX_CHANNELS 1 @@ -153,6 +164,51 @@ enum ad7768_scan_type { AD7768_SCAN_TYPE_HIGH_SPEED, }; +enum { + AD7768_PGA_GAIN_0, + AD7768_PGA_GAIN_1, + AD7768_PGA_GAIN_2, + AD7768_PGA_GAIN_3, + AD7768_PGA_GAIN_4, + AD7768_PGA_GAIN_5, + AD7768_PGA_GAIN_6, + AD7768_PGA_GAIN_7, +}; + +enum { + AD7768_AAF_IN1, + AD7768_AAF_IN2, + AD7768_AAF_IN3, +}; + +/* PGA and AAF gains in V/V */ +static const int adaq7768_gains[] = { + [AD7768_PGA_GAIN_0] = 325, /* 0.325 */ + [AD7768_PGA_GAIN_1] = 650, /* 0.650 */ + [AD7768_PGA_GAIN_2] = 1300, /* 1.300 */ + [AD7768_PGA_GAIN_3] = 2600, /* 2.600 */ + [AD7768_PGA_GAIN_4] = 5200, /* 5.200 */ + [AD7768_PGA_GAIN_5] = 10400, /* 10.400 */ + [AD7768_PGA_GAIN_6] = 20800, /* 20.800 */ +}; + +static const int adaq7769_gains[] = { + [AD7768_PGA_GAIN_0] = 1000, /* 1.000 */ + [AD7768_PGA_GAIN_1] = 2000, /* 2.000 */ + [AD7768_PGA_GAIN_2] = 4000, /* 4.000 */ + [AD7768_PGA_GAIN_3] = 8000, /* 8.000 */ + [AD7768_PGA_GAIN_4] = 16000, /* 16.000 */ + [AD7768_PGA_GAIN_5] = 32000, /* 32.000 */ + [AD7768_PGA_GAIN_6] = 64000, /* 64.000 */ + [AD7768_PGA_GAIN_7] = 128000, /* 128.000 */ +}; + +static const int ad7768_aaf_gains_bp[] = { + [AD7768_AAF_IN1] = 10000, /* 1.000 */ + [AD7768_AAF_IN2] = 3640, /* 0.364 */ + [AD7768_AAF_IN3] = 1430, /* 0.143 */ +}; + /* -3dB cutoff frequency multipliers (relative to ODR) for each filter type. */ static const int ad7768_filter_3db_odr_multiplier[] = { [AD7768_FILTER_SINC5] = 204, /* 0.204 */ @@ -217,6 +273,13 @@ struct ad7768_chip_info { const char *name; const struct iio_chan_spec *channel_spec; int num_channels; + const int *pga_gains; + int num_pga_modes; + int default_pga_mode; + int pgia_mode2pin_offset; + bool has_pga; + bool has_variable_aaf; + bool has_vcm_regulator; }; struct ad7768_state { @@ -234,6 +297,9 @@ struct ad7768_state { unsigned int samp_freq; unsigned int samp_freq_avail[ARRAY_SIZE(ad7768_mclk_div_rates)]; unsigned int samp_freq_avail_len; + unsigned int pga_gain_mode; + unsigned int aaf_gain; + int scale_tbl[ADAQ776X_MAX_GAIN_MODES][2]; struct completion completion; struct iio_trigger *trig; struct gpio_desc *gpio_sync_in; @@ -242,6 +308,7 @@ struct ad7768_state { struct gpio_chip gpiochip; const struct ad7768_chip_info *chip; bool en_spi_sync; + struct mutex pga_lock; /* protect device internal state (PGA) */ /* * DMA (thus cache coherency maintenance) may require the * transfer buffers to live in their own cache lines. @@ -464,6 +531,42 @@ static int ad7768_reg_access(struct iio_dev *indio_dev, return ret; } +static void ad7768_fill_scale_tbl(struct iio_dev *dev) +{ + struct ad7768_state *st = iio_priv(dev); + const struct iio_scan_type *scan_type; + int val, val2, tmp0, tmp1, i; + struct u64_fract fract; + unsigned long n, d; + u64 tmp2; + + scan_type = iio_get_current_scan_type(dev, &dev->channels[0]); + if (scan_type->sign == 's') + val2 = scan_type->realbits - 1; + else + val2 = scan_type->realbits; + + for (i = 0; i < st->chip->num_pga_modes; i++) { + /* Convert gain to a fraction format */ + fract.numerator = st->chip->pga_gains[i]; + fract.denominator = MILLI; + if (st->chip->has_variable_aaf) { + fract.numerator *= ad7768_aaf_gains_bp[st->aaf_gain]; + fract.denominator *= 10000; + } + + rational_best_approximation(fract.numerator, fract.denominator, + INT_MAX, INT_MAX, &n, &d); + + val = mult_frac(st->vref_uv, d, n); + /* Would multiply by NANO here, but value is already in milli */ + tmp2 = shift_right((u64)val * MICRO, val2); + tmp0 = div_u64_rem(tmp2, NANO, &tmp1); + st->scale_tbl[i][0] = tmp0; /* Integer part */ + st->scale_tbl[i][1] = abs(tmp1); /* Fractional part */ + } +} + static int ad7768_set_sinc3_dec_rate(struct ad7768_state *st, unsigned int dec_rate) { @@ -565,12 +668,56 @@ static int ad7768_configure_dig_fil(struct iio_dev *dev, st->oversampling_ratio = ad7768_dec_rate_values[dec_rate_idx]; } + /* Update scale table: scale values vary according to the precision */ + ad7768_fill_scale_tbl(dev); + ad7768_fill_samp_freq_tbl(st); /* A sync-in pulse is required after every configuration change */ return ad7768_send_sync_pulse(st); } +static int ad7768_calc_pga_gain(struct ad7768_state *st, int gain_int, + int gain_fract, int precision) +{ + u64 gain_nano; + u32 tmp; + + gain_nano = gain_int * NANO + gain_fract; + gain_nano = clamp(gain_nano, 0, ADAQ776X_GAIN_MAX_NANO); + tmp = DIV_ROUND_CLOSEST_ULL(gain_nano << precision, NANO); + gain_nano = DIV_ROUND_CLOSEST(st->vref_uv, tmp); + if (st->chip->has_variable_aaf) + gain_nano = DIV_ROUND_CLOSEST_ULL(gain_nano * 10000, + ad7768_aaf_gains_bp[st->aaf_gain]); + + return find_closest(gain_nano, st->chip->pga_gains, + (int)st->chip->num_pga_modes); +} + +static int ad7768_set_pga_gain(struct ad7768_state *st, + int gain_mode) +{ + int pgia_pins_value = abs(gain_mode - st->chip->pgia_mode2pin_offset); + int ret; + + guard(mutex)(&st->pga_lock); + + ret = regmap_write(st->regmap, AD7768_REG_GPIO_CONTROL, AD7768_GPIO_PGIA_EN); + if (ret) + return ret; + + /* Write the respective gain values to GPIOs 0, 1, 2 */ + ret = regmap_write(st->regmap, AD7768_REG_GPIO_WRITE, + AD7768_GPIO_WRITE(pgia_pins_value)); + if (ret) + return ret; + + st->pga_gain_mode = gain_mode; + + return 0; +} + static int ad7768_gpio_direction_input(struct gpio_chip *chip, unsigned int offset) { struct iio_dev *indio_dev = gpiochip_get_data(chip); @@ -778,6 +925,10 @@ static const struct iio_chan_spec ad7768_channels[] = { AD7768_CHAN(0, 0), }; +static const struct iio_chan_spec adaq776x_channels[] = { + AD7768_CHAN(0, BIT(IIO_CHAN_INFO_SCALE)), +}; + static int ad7768_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan, int *val, int *val2, long info) @@ -805,7 +956,19 @@ static int ad7768_read_raw(struct iio_dev *indio_dev, return IIO_VAL_INT; case IIO_CHAN_INFO_SCALE: - *val = (st->vref_uv * 2) / 1000; + if (st->chip->has_pga) { + guard(mutex)(&st->pga_lock); + + *val = st->scale_tbl[st->pga_gain_mode][0]; + *val2 = st->scale_tbl[st->pga_gain_mode][1]; + return IIO_VAL_INT_PLUS_NANO; + } + + temp = (st->vref_uv * 2) / 1000; + if (st->chip->has_variable_aaf) + temp = (temp * 10000) / ad7768_aaf_gains_bp[st->aaf_gain]; + + *val = temp; *val2 = scan_type->realbits; return IIO_VAL_FRACTIONAL_LOG2; @@ -861,18 +1024,39 @@ static int ad7768_read_avail(struct iio_dev *indio_dev, *length = st->samp_freq_avail_len; *type = IIO_VAL_INT; return IIO_AVAIL_LIST; + case IIO_CHAN_INFO_SCALE: + *vals = (int *)st->scale_tbl; + *length = st->chip->num_pga_modes * 2; + *type = IIO_VAL_INT_PLUS_NANO; + return IIO_AVAIL_LIST; default: return -EINVAL; } } +static int ad7768_write_raw_get_fmt(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, long mask) +{ + switch (mask) { + case IIO_CHAN_INFO_SCALE: + return IIO_VAL_INT_PLUS_NANO; + default: + return IIO_VAL_INT_PLUS_MICRO; + } +} + static int __ad7768_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan, int val, int val2, long info) { struct ad7768_state *st = iio_priv(indio_dev); + const struct iio_scan_type *scan_type; int ret; + scan_type = iio_get_current_scan_type(indio_dev, chan); + if (IS_ERR(scan_type)) + return PTR_ERR(scan_type); + switch (info) { case IIO_CHAN_INFO_SAMP_FREQ: return ad7768_set_freq(st, val); @@ -884,6 +1068,21 @@ static int __ad7768_write_raw(struct iio_dev *indio_dev, /* Update sampling frequency */ return ad7768_set_freq(st, st->samp_freq); + case IIO_CHAN_INFO_SCALE: { + int gain_mode; + + if (!st->chip->has_pga) + return -EOPNOTSUPP; + + if (scan_type->sign == 's') + gain_mode = ad7768_calc_pga_gain(st, val, val2, + scan_type->realbits - 1); + else + gain_mode = ad7768_calc_pga_gain(st, val, val2, + scan_type->realbits); + + return ad7768_set_pga_gain(st, gain_mode); + } default: return -EINVAL; } @@ -925,6 +1124,7 @@ static const struct iio_info ad7768_info = { .read_raw = &ad7768_read_raw, .read_avail = &ad7768_read_avail, .write_raw = &ad7768_write_raw, + .write_raw_get_fmt = &ad7768_write_raw_get_fmt, .read_label = ad7768_read_label, .get_current_scan_type = &ad7768_get_current_scan_type, .debugfs_reg_access = &ad7768_reg_access, @@ -1331,10 +1531,80 @@ static int ad7768_register_regulators(struct device *dev, struct ad7768_state *s return 0; } +static int ad7768_parse_aaf_gain(struct device *dev, struct ad7768_state *st) +{ + bool aaf_gain_provided; + u32 val; + int ret; + + st->aaf_gain = AD7768_AAF_IN1; + ret = device_property_read_u32(dev, "adi,aaf-gain-bp", &val); + aaf_gain_provided = (ret == 0); + if (!aaf_gain_provided && st->chip->has_variable_aaf) { + dev_warn(dev, "AAF gain not specified, using default\n"); + return 0; + } + + if (aaf_gain_provided && !st->chip->has_variable_aaf) { + dev_warn(dev, "AAF gain not supported for %s\n", st->chip->name); + return 0; + } + + if (aaf_gain_provided) { + switch (val) { + case 10000: + st->aaf_gain = AD7768_AAF_IN1; + break; + case 3640: + st->aaf_gain = AD7768_AAF_IN2; + break; + case 1430: + st->aaf_gain = AD7768_AAF_IN3; + break; + default: + return dev_err_probe(dev, -EINVAL, + "Invalid firmware provided AAF gain\n"); + } + } + + return 0; +} + static const struct ad7768_chip_info ad7768_chip_info = { .name = "ad7768-1", .channel_spec = ad7768_channels, .num_channels = ARRAY_SIZE(ad7768_channels), + .has_vcm_regulator = true, +}; + +static const struct ad7768_chip_info adaq7767_chip_info = { + .name = "adaq7767-1", + .channel_spec = ad7768_channels, + .num_channels = ARRAY_SIZE(ad7768_channels), + .has_variable_aaf = true, +}; + +static const struct ad7768_chip_info adaq7768_chip_info = { + .name = "adaq7768-1", + .channel_spec = adaq776x_channels, + .num_channels = ARRAY_SIZE(adaq776x_channels), + .pga_gains = adaq7768_gains, + .default_pga_mode = AD7768_PGA_GAIN_2, + .num_pga_modes = ARRAY_SIZE(adaq7768_gains), + .pgia_mode2pin_offset = 6, + .has_pga = true, +}; + +static const struct ad7768_chip_info adaq7769_chip_info = { + .name = "adaq7769-1", + .channel_spec = adaq776x_channels, + .num_channels = ARRAY_SIZE(adaq776x_channels), + .pga_gains = adaq7769_gains, + .default_pga_mode = AD7768_PGA_GAIN_0, + .num_pga_modes = ARRAY_SIZE(adaq7769_gains), + .pgia_mode2pin_offset = 0, + .has_pga = true, + .has_variable_aaf = true, }; static int ad7768_probe(struct spi_device *spi) @@ -1399,7 +1669,13 @@ static int ad7768_probe(struct spi_device *spi) indio_dev->modes = INDIO_DIRECT_MODE; /* Register VCM output regulator */ - ret = ad7768_register_regulators(&spi->dev, st, indio_dev); + if (st->chip->has_vcm_regulator) { + ret = ad7768_register_regulators(&spi->dev, st, indio_dev); + if (ret) + return ret; + } + + ret = ad7768_parse_aaf_gain(&spi->dev, st); if (ret) return ret; @@ -1410,6 +1686,12 @@ static int ad7768_probe(struct spi_device *spi) } init_completion(&st->completion); + ret = devm_mutex_init(&spi->dev, &st->pga_lock); + if (ret) + return ret; + + if (st->chip->has_pga) + ad7768_set_pga_gain(st, st->chip->default_pga_mode); ret = ad7768_set_channel_label(indio_dev, st->chip->num_channels); if (ret) @@ -1431,12 +1713,18 @@ static int ad7768_probe(struct spi_device *spi) static const struct spi_device_id ad7768_id_table[] = { { "ad7768-1", (kernel_ulong_t)&ad7768_chip_info }, + { "adaq7767-1", (kernel_ulong_t)&adaq7767_chip_info }, + { "adaq7768-1", (kernel_ulong_t)&adaq7768_chip_info }, + { "adaq7769-1", (kernel_ulong_t)&adaq7769_chip_info }, { } }; MODULE_DEVICE_TABLE(spi, ad7768_id_table); static const struct of_device_id ad7768_of_match[] = { { .compatible = "adi,ad7768-1", .data = &ad7768_chip_info }, + { .compatible = "adi,adaq7767-1", .data = &adaq7767_chip_info }, + { .compatible = "adi,adaq7768-1", .data = &adaq7768_chip_info }, + { .compatible = "adi,adaq7769-1", .data = &adaq7769_chip_info }, { } }; MODULE_DEVICE_TABLE(of, ad7768_of_match); -- 2.34.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 4/4] iio: adc: ad7768-1: add support for ADAQ776x-1 ADC Family 2025-09-05 9:49 ` [PATCH v3 4/4] iio: adc: ad7768-1: add support for ADAQ776x-1 ADC Family Jonathan Santos @ 2025-09-05 17:08 ` David Lechner 2025-09-05 17:23 ` Andy Shevchenko 1 sibling, 0 replies; 13+ messages in thread From: David Lechner @ 2025-09-05 17:08 UTC (permalink / raw) To: Jonathan Santos, linux-iio, devicetree, linux-kernel Cc: lars, jic23, nuno.sa, andy, robh, krzk+dt, conor+dt, marcelo.schmitt1, jonath4nns On 9/5/25 4:49 AM, Jonathan Santos wrote: > Add support for ADAQ7767/68/69-1 series, which includes PGIA and > Anti-aliasing filter (AAF) gains. Unlike the AD7768-1, they do not > provide a VCM regulator interface. > ... > +static void ad7768_fill_scale_tbl(struct iio_dev *dev) > +{ > + struct ad7768_state *st = iio_priv(dev); > + const struct iio_scan_type *scan_type; > + int val, val2, tmp0, tmp1, i; > + struct u64_fract fract; > + unsigned long n, d; > + u64 tmp2; > + > + scan_type = iio_get_current_scan_type(dev, &dev->channels[0]); > + if (scan_type->sign == 's') > + val2 = scan_type->realbits - 1; > + else > + val2 = scan_type->realbits; > + > + for (i = 0; i < st->chip->num_pga_modes; i++) { > + /* Convert gain to a fraction format */ > + fract.numerator = st->chip->pga_gains[i]; > + fract.denominator = MILLI; > + if (st->chip->has_variable_aaf) { > + fract.numerator *= ad7768_aaf_gains_bp[st->aaf_gain]; > + fract.denominator *= 10000; > + } > + > + rational_best_approximation(fract.numerator, fract.denominator, > + INT_MAX, INT_MAX, &n, &d); > + > + val = mult_frac(st->vref_uv, d, n); > + /* Would multiply by NANO here, but value is already in milli */ > + tmp2 = shift_right((u64)val * MICRO, val2); shift_right() is only needed for signed values. Can just use >> since this is dealing with unsigned. > + tmp0 = div_u64_rem(tmp2, NANO, &tmp1); > + st->scale_tbl[i][0] = tmp0; /* Integer part */ > + st->scale_tbl[i][1] = abs(tmp1); /* Fractional part */ > + } > +} > + ... > +static int ad7768_set_pga_gain(struct ad7768_state *st, > + int gain_mode) > +{ > + int pgia_pins_value = abs(gain_mode - st->chip->pgia_mode2pin_offset); > + int ret; > + > + guard(mutex)(&st->pga_lock); > + > + ret = regmap_write(st->regmap, AD7768_REG_GPIO_CONTROL, AD7768_GPIO_PGIA_EN); Hiding multiple fields in a macro makes it harder to see what this is doing. > + if (ret) > + return ret; > + > + /* Write the respective gain values to GPIOs 0, 1, 2 */ > + ret = regmap_write(st->regmap, AD7768_REG_GPIO_WRITE, > + AD7768_GPIO_WRITE(pgia_pins_value)); Hiding the FIELD_PREP() in a macro makes this a bit harder to understand. > + if (ret) > + return ret; > + > + st->pga_gain_mode = gain_mode; > + > + return 0; > +} > + > static int ad7768_gpio_direction_input(struct gpio_chip *chip, unsigned int offset) > { > struct iio_dev *indio_dev = gpiochip_get_data(chip); > @@ -778,6 +925,10 @@ static const struct iio_chan_spec ad7768_channels[] = { > AD7768_CHAN(0, 0), > }; > > +static const struct iio_chan_spec adaq776x_channels[] = { > + AD7768_CHAN(0, BIT(IIO_CHAN_INFO_SCALE)), > +}; > + > static int ad7768_read_raw(struct iio_dev *indio_dev, > struct iio_chan_spec const *chan, > int *val, int *val2, long info) > @@ -805,7 +956,19 @@ static int ad7768_read_raw(struct iio_dev *indio_dev, > return IIO_VAL_INT; > > case IIO_CHAN_INFO_SCALE: > - *val = (st->vref_uv * 2) / 1000; > + if (st->chip->has_pga) { > + guard(mutex)(&st->pga_lock); > + > + *val = st->scale_tbl[st->pga_gain_mode][0]; > + *val2 = st->scale_tbl[st->pga_gain_mode][1]; > + return IIO_VAL_INT_PLUS_NANO; > + } > + > + temp = (st->vref_uv * 2) / 1000; > + if (st->chip->has_variable_aaf) > + temp = (temp * 10000) / ad7768_aaf_gains_bp[st->aaf_gain]; Should we add a BASIS_POINTS macro to units.h since we are calling this a standard unit? > + > + *val = temp; > *val2 = scan_type->realbits; > > return IIO_VAL_FRACTIONAL_LOG2; ... > +static int ad7768_parse_aaf_gain(struct device *dev, struct ad7768_state *st) > +{ > + bool aaf_gain_provided; > + u32 val; > + int ret; > + > + st->aaf_gain = AD7768_AAF_IN1; No sense in setting this if we return early on -EINVAL. > + ret = device_property_read_u32(dev, "adi,aaf-gain-bp", &val); > + aaf_gain_provided = (ret == 0); Only -EINVAL means the property is not present. Other errors can be returned. Or add a comment explaining why we don't care about other errors. > + if (!aaf_gain_provided && st->chip->has_variable_aaf) { > + dev_warn(dev, "AAF gain not specified, using default\n"); Using the default seems like a normal thing to do, so should not be a warning. > + return 0; > + } > + > + if (aaf_gain_provided && !st->chip->has_variable_aaf) { > + dev_warn(dev, "AAF gain not supported for %s\n", st->chip->name); Returning error here would be senseible. Clearly there is something wrong with the devicetree. But it could be something else, like the wrong compaible string. > + return 0; > + } > + > + if (aaf_gain_provided) { > + switch (val) { > + case 10000: > + st->aaf_gain = AD7768_AAF_IN1; > + break; > + case 3640: > + st->aaf_gain = AD7768_AAF_IN2; > + break; > + case 1430: > + st->aaf_gain = AD7768_AAF_IN3; > + break; > + default: > + return dev_err_probe(dev, -EINVAL, > + "Invalid firmware provided AAF gain\n"); > + } > + } > + > + return 0; > +} > + ... > static int ad7768_probe(struct spi_device *spi) > @@ -1399,7 +1669,13 @@ static int ad7768_probe(struct spi_device *spi) > indio_dev->modes = INDIO_DIRECT_MODE; > > /* Register VCM output regulator */ > - ret = ad7768_register_regulators(&spi->dev, st, indio_dev); > + if (st->chip->has_vcm_regulator) { > + ret = ad7768_register_regulators(&spi->dev, st, indio_dev); ad7768_register_regulators() now seems mis-named since it only handles VMC supply. > + if (ret) > + return ret; > + } > + > + ret = ad7768_parse_aaf_gain(&spi->dev, st); > if (ret) > return ret; > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 4/4] iio: adc: ad7768-1: add support for ADAQ776x-1 ADC Family 2025-09-05 9:49 ` [PATCH v3 4/4] iio: adc: ad7768-1: add support for ADAQ776x-1 ADC Family Jonathan Santos 2025-09-05 17:08 ` David Lechner @ 2025-09-05 17:23 ` Andy Shevchenko 2025-09-05 17:25 ` Andy Shevchenko 2025-09-07 10:56 ` Jonathan Cameron 1 sibling, 2 replies; 13+ messages in thread From: Andy Shevchenko @ 2025-09-05 17:23 UTC (permalink / raw) To: Jonathan Santos Cc: linux-iio, devicetree, linux-kernel, lars, jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, marcelo.schmitt1, jonath4nns On Fri, Sep 05, 2025 at 06:49:42AM -0300, Jonathan Santos wrote: > Add support for ADAQ7767/68/69-1 series, which includes PGIA and > Anti-aliasing filter (AAF) gains. Unlike the AD7768-1, they do not > provide a VCM regulator interface. > > The PGA gain is configured in run-time through the scale attribute, > if supported by the device. PGA is enabled and controlled by the GPIO > controller (GPIOs 0, 1 and 2) provided by the device with the SPI > interface. > > The AAF gain is defined by hardware connections and should be specified > in the device tree. ... > +static void ad7768_fill_scale_tbl(struct iio_dev *dev) > +{ > + struct ad7768_state *st = iio_priv(dev); > + const struct iio_scan_type *scan_type; > + int val, val2, tmp0, tmp1, i; > + struct u64_fract fract; > + unsigned long n, d; > + u64 tmp2; > + > + scan_type = iio_get_current_scan_type(dev, &dev->channels[0]); Is it usual patter in IIO? Otherwise it can be written as scan_type = iio_get_current_scan_type(dev, dev->channels); > + if (scan_type->sign == 's') > + val2 = scan_type->realbits - 1; > + else > + val2 = scan_type->realbits; Wasn't smatch happy about this check? > + for (i = 0; i < st->chip->num_pga_modes; i++) { > + /* Convert gain to a fraction format */ > + fract.numerator = st->chip->pga_gains[i]; > + fract.denominator = MILLI; > + if (st->chip->has_variable_aaf) { > + fract.numerator *= ad7768_aaf_gains_bp[st->aaf_gain]; > + fract.denominator *= 10000; > + } My question also was: Are these overflow u32? If not, the second patch is not needed. Otherwise, how is the previous version supposed to work with unsigned long type (if I am not mistaken) on 32-bit platforms? > + rational_best_approximation(fract.numerator, fract.denominator, > + INT_MAX, INT_MAX, &n, &d); > + > + val = mult_frac(st->vref_uv, d, n); > + /* Would multiply by NANO here, but value is already in milli */ > + tmp2 = shift_right((u64)val * MICRO, val2); > + tmp0 = div_u64_rem(tmp2, NANO, &tmp1); > + st->scale_tbl[i][0] = tmp0; /* Integer part */ > + st->scale_tbl[i][1] = abs(tmp1); /* Fractional part */ > + } > +} ... > +static int ad7768_calc_pga_gain(struct ad7768_state *st, int gain_int, > + int gain_fract, int precision) > +{ > + u64 gain_nano; > + u32 tmp; > + > + gain_nano = gain_int * NANO + gain_fract; > + gain_nano = clamp(gain_nano, 0, ADAQ776X_GAIN_MAX_NANO); > + tmp = DIV_ROUND_CLOSEST_ULL(gain_nano << precision, NANO); > + gain_nano = DIV_ROUND_CLOSEST(st->vref_uv, tmp); > + if (st->chip->has_variable_aaf) > + gain_nano = DIV_ROUND_CLOSEST_ULL(gain_nano * 10000, One space too many. > + ad7768_aaf_gains_bp[st->aaf_gain]); > + > + return find_closest(gain_nano, st->chip->pga_gains, > + (int)st->chip->num_pga_modes); Is casting needed? > +} ... > + case IIO_CHAN_INFO_SCALE: { One space too many. > + int gain_mode; > + > + if (!st->chip->has_pga) > + return -EOPNOTSUPP; > + > + if (scan_type->sign == 's') > + gain_mode = ad7768_calc_pga_gain(st, val, val2, > + scan_type->realbits - 1); > + else > + gain_mode = ad7768_calc_pga_gain(st, val, val2, > + scan_type->realbits); > + > + return ad7768_set_pga_gain(st, gain_mode); > + } ... > +static int ad7768_parse_aaf_gain(struct device *dev, struct ad7768_state *st) > +{ > + bool aaf_gain_provided; > + u32 val; > + int ret; > + st->aaf_gain = AD7768_AAF_IN1; > + ret = device_property_read_u32(dev, "adi,aaf-gain-bp", &val); > + aaf_gain_provided = (ret == 0); Hmm... It seems rather incorrect check. You should actually return an error code in case it's provided but can't be read. I would expect something like ret = device_property_read_u32(dev, "adi,aaf-gain-bp", &val); if (ret == -EINVAL) _gain_provided = false; else if (ret) return dev_err_probe(...); else _gain_provided = true; > + if (!aaf_gain_provided && st->chip->has_variable_aaf) { > + dev_warn(dev, "AAF gain not specified, using default\n"); > + return 0; > + } > + > + if (aaf_gain_provided && !st->chip->has_variable_aaf) { > + dev_warn(dev, "AAF gain not supported for %s\n", st->chip->name); > + return 0; > + } We can refactor these to have one level indentation less for the switch-case. if (aaf_gain_provided && !st->chip->has_variable_aaf) { dev_warn(dev, "AAF gain not supported for %s\n", st->chip->name); return 0; } if (!aaf_gain_provided) { if (st->chip->has_variable_aaf) dev_warn(dev, "AAF gain not specified, using default\n"); return 0; } > + if (aaf_gain_provided) { So this one may be dropped. > + switch (val) { > + case 10000: > + st->aaf_gain = AD7768_AAF_IN1; > + break; > + case 3640: > + st->aaf_gain = AD7768_AAF_IN2; > + break; > + case 1430: > + st->aaf_gain = AD7768_AAF_IN3; > + break; > + default: > + return dev_err_probe(dev, -EINVAL, > + "Invalid firmware provided AAF gain\n"); > + } > + } > + return 0; > +} -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 4/4] iio: adc: ad7768-1: add support for ADAQ776x-1 ADC Family 2025-09-05 17:23 ` Andy Shevchenko @ 2025-09-05 17:25 ` Andy Shevchenko 2025-09-07 10:56 ` Jonathan Cameron 1 sibling, 0 replies; 13+ messages in thread From: Andy Shevchenko @ 2025-09-05 17:25 UTC (permalink / raw) To: Jonathan Santos Cc: linux-iio, devicetree, linux-kernel, lars, jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, marcelo.schmitt1, jonath4nns On Fri, Sep 05, 2025 at 08:23:33PM +0300, Andy Shevchenko wrote: > On Fri, Sep 05, 2025 at 06:49:42AM -0300, Jonathan Santos wrote: ... > > + if (scan_type->sign == 's') > > + val2 = scan_type->realbits - 1; > > + else > > + val2 = scan_type->realbits; > > Wasn't smatch happy about this check? I meant unhappy -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 4/4] iio: adc: ad7768-1: add support for ADAQ776x-1 ADC Family 2025-09-05 17:23 ` Andy Shevchenko 2025-09-05 17:25 ` Andy Shevchenko @ 2025-09-07 10:56 ` Jonathan Cameron 1 sibling, 0 replies; 13+ messages in thread From: Jonathan Cameron @ 2025-09-07 10:56 UTC (permalink / raw) To: Andy Shevchenko Cc: Jonathan Santos, linux-iio, devicetree, linux-kernel, lars, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, marcelo.schmitt1, jonath4nns > > +static void ad7768_fill_scale_tbl(struct iio_dev *dev) > > +{ > > + struct ad7768_state *st = iio_priv(dev); > > + const struct iio_scan_type *scan_type; > > + int val, val2, tmp0, tmp1, i; > > + struct u64_fract fract; > > + unsigned long n, d; > > + u64 tmp2; > > + > > + scan_type = iio_get_current_scan_type(dev, &dev->channels[0]); > > Is it usual patter in IIO? Otherwise it can be written as > > scan_type = iio_get_current_scan_type(dev, dev->channels); From a semantic / readability point of view I'd keep it referencing the first element. We are querying the scan type of one specific channel, rather than the array that is behind dev->channels. > > > + if (scan_type->sign == 's') > > + val2 = scan_type->realbits - 1; > > + else > > + val2 = scan_type->realbits; ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-09-07 10:56 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-09-05 9:48 [PATCH v3 0/4] Add support for ADAQ776x-1 ADC Family Jonathan Santos 2025-09-05 9:49 ` [PATCH v3 1/4] dt-bindings: iio: adc: ad7768-1: add new supported parts Jonathan Santos 2025-09-05 17:06 ` David Lechner 2025-09-05 9:49 ` [PATCH v3 2/4] iio: adc: ad7768-1: introduce chip info for future multidevice support Jonathan Santos 2025-09-05 16:51 ` Andy Shevchenko 2025-09-05 17:06 ` David Lechner 2025-09-05 9:49 ` [PATCH v3 3/4] math.h: Add 64-bit fractional numbers types Jonathan Santos 2025-09-05 16:45 ` Andy Shevchenko 2025-09-05 9:49 ` [PATCH v3 4/4] iio: adc: ad7768-1: add support for ADAQ776x-1 ADC Family Jonathan Santos 2025-09-05 17:08 ` David Lechner 2025-09-05 17:23 ` Andy Shevchenko 2025-09-05 17:25 ` Andy Shevchenko 2025-09-07 10:56 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).