* [PATCH v3 1/8] dt-bindings: iio: adc: adi,ad4030: Reference spi-peripheral-props
2025-09-26 20:37 [PATCH v3 0/8] Add SPI offload support to AD4030 Marcelo Schmitt
@ 2025-09-26 20:38 ` Marcelo Schmitt
2025-09-26 20:38 ` [PATCH v3 2/8] Docs: iio: ad4030: Add double PWM SPI offload doc Marcelo Schmitt
` (6 subsequent siblings)
7 siblings, 0 replies; 23+ messages in thread
From: Marcelo Schmitt @ 2025-09-26 20:38 UTC (permalink / raw)
To: linux-iio, devicetree, linux-doc, linux-spi, linux-kernel
Cc: jic23, michael.hennerich, nuno.sa, eblanc, dlechner, andy, robh,
krzk+dt, conor+dt, corbet, marcelo.schmitt1, Conor Dooley
AD4030 and similar devices all connect to the system as SPI peripherals.
Reference spi-peripheral-props so common SPI peripheral can be used from
ad4030 dt-binding.
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
Documentation/devicetree/bindings/iio/adc/adi,ad4030.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4030.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4030.yaml
index 54e7349317b7..a8fee4062d0e 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad4030.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4030.yaml
@@ -20,6 +20,8 @@ description: |
* https://www.analog.com/media/en/technical-documentation/data-sheets/ad4630-24_ad4632-24.pdf
* https://www.analog.com/media/en/technical-documentation/data-sheets/ad4630-16-4632-16.pdf
+$ref: /schemas/spi/spi-peripheral-props.yaml#
+
properties:
compatible:
enum:
--
2.39.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v3 2/8] Docs: iio: ad4030: Add double PWM SPI offload doc
2025-09-26 20:37 [PATCH v3 0/8] Add SPI offload support to AD4030 Marcelo Schmitt
2025-09-26 20:38 ` [PATCH v3 1/8] dt-bindings: iio: adc: adi,ad4030: Reference spi-peripheral-props Marcelo Schmitt
@ 2025-09-26 20:38 ` Marcelo Schmitt
2025-09-26 20:39 ` [PATCH v3 3/8] dt-bindings: iio: adc: adi,ad4030: Add PWM Marcelo Schmitt
` (5 subsequent siblings)
7 siblings, 0 replies; 23+ messages in thread
From: Marcelo Schmitt @ 2025-09-26 20:38 UTC (permalink / raw)
To: linux-iio, devicetree, linux-doc, linux-spi, linux-kernel
Cc: jic23, michael.hennerich, nuno.sa, eblanc, dlechner, andy, robh,
krzk+dt, conor+dt, corbet, marcelo.schmitt1
Document double PWM setup SPI offload wiring schema.
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
Change log v2 -> v3
- Picked up reviewed-by tags
- Updated AD4030 documentation with link to a working project that supports
double PWM setup.
Documentation/iio/ad4030.rst | 39 ++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/Documentation/iio/ad4030.rst b/Documentation/iio/ad4030.rst
index b57424b650a8..9caafa4148b0 100644
--- a/Documentation/iio/ad4030.rst
+++ b/Documentation/iio/ad4030.rst
@@ -92,6 +92,45 @@ Interleaved mode
In this mode, both channels conversion results are bit interleaved one SDO line.
As such the wiring is the same as `One lane mode`_.
+SPI offload wiring
+^^^^^^^^^^^^^^^^^^
+
+.. code-block::
+
+ +-------------+ +-------------+
+ | CNV |<-----+--| GPIO |
+ | | +--| PWM0 |
+ | | | |
+ | | +--| PWM1 |
+ | | | +-------------+
+ | | +->| TRIGGER |
+ | CS |<--------| CS |
+ | | | |
+ | ADC | | SPI |
+ | | | |
+ | SDI |<--------| SDO |
+ | SDO |-------->| SDI |
+ | SCLK |<--------| SCLK |
+ +-------------+ +-------------+
+
+In this mode, both the ``cnv-gpios`` and a ``pwms`` properties are required.
+The ``pwms`` property specifies the PWM that is connected to the ADC CNV pin.
+The SPI offload will have a ``trigger-sources`` property to indicate the SPI
+offload (PWM) trigger source. For AD4030 and similar ADCs, there are two
+possible data transfer zones for sample N. One of them (zone 1) starts after the
+data conversion for sample N is complete while the other one (zone 2) starts 9.8
+nanoseconds after the rising edge of CNV for sample N + 1.
+
+The configuration depicted in the above diagram is intended to perform data
+transfer in zone 2. To achieve high sample rates while meeting ADC timing
+requirements, an offset is added between the rising edges of PWM0 and PWM1 to
+delay the SPI transfer until 9.8 nanoseconds after CNV rising edge. This
+requires a specialized PWM controller that can provide such an offset.
+The `AD4630-FMC HDL project`_, for example, can be configured to sample AD4030
+data during zone 2 data read window.
+
+.. _AD4630-FMC HDL project: https://analogdevicesinc.github.io/hdl/projects/ad4630_fmc/index.html
+
SPI Clock mode
--------------
--
2.39.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v3 3/8] dt-bindings: iio: adc: adi,ad4030: Add PWM
2025-09-26 20:37 [PATCH v3 0/8] Add SPI offload support to AD4030 Marcelo Schmitt
2025-09-26 20:38 ` [PATCH v3 1/8] dt-bindings: iio: adc: adi,ad4030: Reference spi-peripheral-props Marcelo Schmitt
2025-09-26 20:38 ` [PATCH v3 2/8] Docs: iio: ad4030: Add double PWM SPI offload doc Marcelo Schmitt
@ 2025-09-26 20:39 ` Marcelo Schmitt
2025-09-26 20:39 ` [PATCH v3 4/8] iio: adc: ad4030: Reduce register access transfer speed Marcelo Schmitt
` (4 subsequent siblings)
7 siblings, 0 replies; 23+ messages in thread
From: Marcelo Schmitt @ 2025-09-26 20:39 UTC (permalink / raw)
To: linux-iio, devicetree, linux-doc, linux-spi, linux-kernel
Cc: jic23, michael.hennerich, nuno.sa, eblanc, dlechner, andy, robh,
krzk+dt, conor+dt, corbet, marcelo.schmitt1, Conor Dooley
In setups designed for high speed data rate capture, a PWM is used to
generate the CNV signal that issues data captures from the ADC. Document
the use of a PWM for AD4030 and similar devices.
Reviewed-by: David Lechner <dlechner@baylibre.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
Documentation/devicetree/bindings/iio/adc/adi,ad4030.yaml | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4030.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4030.yaml
index a8fee4062d0e..564b6f67a96e 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad4030.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4030.yaml
@@ -64,6 +64,10 @@ properties:
The Reset Input (/RST). Used for asynchronous device reset.
maxItems: 1
+ pwms:
+ description: PWM signal connected to the CNV pin.
+ maxItems: 1
+
interrupts:
description:
The BUSY pin is used to signal that the conversions results are available
--
2.39.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v3 4/8] iio: adc: ad4030: Reduce register access transfer speed
2025-09-26 20:37 [PATCH v3 0/8] Add SPI offload support to AD4030 Marcelo Schmitt
` (2 preceding siblings ...)
2025-09-26 20:39 ` [PATCH v3 3/8] dt-bindings: iio: adc: adi,ad4030: Add PWM Marcelo Schmitt
@ 2025-09-26 20:39 ` Marcelo Schmitt
2025-09-28 9:53 ` Jonathan Cameron
2025-09-26 20:40 ` [PATCH v3 5/8] iio: adc: ad4030: Use BIT macro to improve code readability Marcelo Schmitt
` (3 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Marcelo Schmitt @ 2025-09-26 20:39 UTC (permalink / raw)
To: linux-iio, devicetree, linux-doc, linux-spi, linux-kernel
Cc: jic23, michael.hennerich, nuno.sa, eblanc, dlechner, andy, robh,
krzk+dt, conor+dt, corbet, marcelo.schmitt1
Configuration register accesses are not considered a critical path in terms
of time to complete. Even though register access transfers can run at high
speeds, nanosecond completion times are not required as device
configuration is usually done step by step from user space. Also, high
frequency transfers hinder debug with external tools since they require
faster clocked equipment. Reduce register access transfer speed.
Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
drivers/iio/adc/ad4030.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/iio/adc/ad4030.c b/drivers/iio/adc/ad4030.c
index 1bc2f9a22470..7f0dbb8a3b07 100644
--- a/drivers/iio/adc/ad4030.c
+++ b/drivers/iio/adc/ad4030.c
@@ -102,7 +102,7 @@
#define AD4030_VREF_MAX_UV (5000 * MILLI)
#define AD4030_VIO_THRESHOLD_UV (1400 * MILLI)
#define AD4030_SPI_MAX_XFER_LEN 8
-#define AD4030_SPI_MAX_REG_XFER_SPEED (80 * MEGA)
+#define AD4030_SPI_REG_XFER_SPEED (10 * MEGA)
#define AD4030_TCNVH_NS 10
#define AD4030_TCNVL_NS 20
#define AD4030_TCYC_NS 500
@@ -245,7 +245,7 @@ static int ad4030_enter_config_mode(struct ad4030_state *st)
struct spi_transfer xfer = {
.tx_buf = st->tx_data,
.len = 1,
- .speed_hz = AD4030_SPI_MAX_REG_XFER_SPEED,
+ .speed_hz = AD4030_SPI_REG_XFER_SPEED,
};
return spi_sync_transfer(st->spi, &xfer, 1);
@@ -260,7 +260,7 @@ static int ad4030_exit_config_mode(struct ad4030_state *st)
struct spi_transfer xfer = {
.tx_buf = st->tx_data,
.len = 3,
- .speed_hz = AD4030_SPI_MAX_REG_XFER_SPEED,
+ .speed_hz = AD4030_SPI_REG_XFER_SPEED,
};
return spi_sync_transfer(st->spi, &xfer, 1);
@@ -275,7 +275,7 @@ static int ad4030_spi_read(void *context, const void *reg, size_t reg_size,
.tx_buf = st->tx_data,
.rx_buf = st->rx_data.raw,
.len = reg_size + val_size,
- .speed_hz = AD4030_SPI_MAX_REG_XFER_SPEED,
+ .speed_hz = AD4030_SPI_REG_XFER_SPEED,
};
if (xfer.len > sizeof(st->tx_data) ||
@@ -309,7 +309,7 @@ static int ad4030_spi_write(void *context, const void *data, size_t count)
struct spi_transfer xfer = {
.tx_buf = st->tx_data,
.len = count,
- .speed_hz = AD4030_SPI_MAX_REG_XFER_SPEED,
+ .speed_hz = AD4030_SPI_REG_XFER_SPEED,
};
if (count > sizeof(st->tx_data))
--
2.39.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v3 4/8] iio: adc: ad4030: Reduce register access transfer speed
2025-09-26 20:39 ` [PATCH v3 4/8] iio: adc: ad4030: Reduce register access transfer speed Marcelo Schmitt
@ 2025-09-28 9:53 ` Jonathan Cameron
2025-09-28 14:17 ` Marcelo Schmitt
0 siblings, 1 reply; 23+ messages in thread
From: Jonathan Cameron @ 2025-09-28 9:53 UTC (permalink / raw)
To: Marcelo Schmitt
Cc: linux-iio, devicetree, linux-doc, linux-spi, linux-kernel,
michael.hennerich, nuno.sa, eblanc, dlechner, andy, robh, krzk+dt,
conor+dt, corbet, marcelo.schmitt1
On Fri, 26 Sep 2025 17:39:42 -0300
Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:
> Configuration register accesses are not considered a critical path in terms
> of time to complete. Even though register access transfers can run at high
> speeds, nanosecond completion times are not required as device
> configuration is usually done step by step from user space. Also, high
> frequency transfers hinder debug with external tools since they require
> faster clocked equipment. Reduce register access transfer speed.
So making debug with external tools easier isn't usually a justification we'd
make to slow things down by default.
Is there another reason for this being useful as opposed to not a problem
to do? If it had been done this way in the first place I wouldn't have
minded, but to make a change I'd like either some others to jump in and
say, yes please do this, or a reason beyond you are using tooling that can't
cope with 80 MHz and don't want to hack the driver when you need
to slow it down (my tools can't cope with that rate either!)
J
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 4/8] iio: adc: ad4030: Reduce register access transfer speed
2025-09-28 9:53 ` Jonathan Cameron
@ 2025-09-28 14:17 ` Marcelo Schmitt
0 siblings, 0 replies; 23+ messages in thread
From: Marcelo Schmitt @ 2025-09-28 14:17 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Marcelo Schmitt, linux-iio, devicetree, linux-doc, linux-spi,
linux-kernel, michael.hennerich, nuno.sa, eblanc, dlechner, andy,
robh, krzk+dt, conor+dt, corbet
On 09/28, Jonathan Cameron wrote:
> On Fri, 26 Sep 2025 17:39:42 -0300
> Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:
>
> > Configuration register accesses are not considered a critical path in terms
> > of time to complete. Even though register access transfers can run at high
> > speeds, nanosecond completion times are not required as device
> > configuration is usually done step by step from user space. Also, high
> > frequency transfers hinder debug with external tools since they require
> > faster clocked equipment. Reduce register access transfer speed.
>
> So making debug with external tools easier isn't usually a justification we'd
> make to slow things down by default.
>
> Is there another reason for this being useful as opposed to not a problem
> to do? If it had been done this way in the first place I wouldn't have
> minded, but to make a change I'd like either some others to jump in and
> say, yes please do this, or a reason beyond you are using tooling that can't
> cope with 80 MHz and don't want to hack the driver when you need
> to slow it down (my tools can't cope with that rate either!)
Main motivation for this was a suggestion from David.
https://lore.kernel.org/linux-iio/30659b16-290d-4ae5-a644-214c106bbe87@baylibre.com/
By the way, if he agrees with, I'll add a suggested-by tag (if we decide to keep
this patch).
Reasoning a bit more about this, lowering reg access speed may help debug with
external tools, but it won't help debug transfers ran by SPI offload hw because
those transfers will be fast anyway. Maybe a more relevant potential benefit of
lowering transfer speeds would be to make it more "friendly" to slower
controllers. E.g. raspberry pi controller reaches 32 MHz maximum so, unless SPI
core can adapt transfers in those cases, it wouldn't work on a rpi (if anyone
ever connects this to a rpi).
Me, I only have remote access to a setup with adaq4216 connected to a zedboard
so I won't be connecting any external tool for debugging.
Another thing that came to mind now is we could just not set speed_hz of
spi_transfers. AFAIC, those are not required.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 5/8] iio: adc: ad4030: Use BIT macro to improve code readability
2025-09-26 20:37 [PATCH v3 0/8] Add SPI offload support to AD4030 Marcelo Schmitt
` (3 preceding siblings ...)
2025-09-26 20:39 ` [PATCH v3 4/8] iio: adc: ad4030: Reduce register access transfer speed Marcelo Schmitt
@ 2025-09-26 20:40 ` Marcelo Schmitt
2025-09-26 20:40 ` [PATCH v3 6/8] iio: adc: ad4030: Add SPI offload support Marcelo Schmitt
` (2 subsequent siblings)
7 siblings, 0 replies; 23+ messages in thread
From: Marcelo Schmitt @ 2025-09-26 20:40 UTC (permalink / raw)
To: linux-iio, devicetree, linux-doc, linux-spi, linux-kernel
Cc: jic23, michael.hennerich, nuno.sa, eblanc, dlechner, andy, robh,
krzk+dt, conor+dt, corbet, marcelo.schmitt1, Andy Shevchenko
Use BIT macro to make the list of average modes more readable.
Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Link: https://lore.kernel.org/linux-iio/CAHp75Vfu-C3Hd0ZXTj4rxEgRe_O84cfo6jiRCPFxZJnYrvROWQ@mail.gmail.com/
Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
drivers/iio/adc/ad4030.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/adc/ad4030.c b/drivers/iio/adc/ad4030.c
index 7f0dbb8a3b07..cdf5933e9725 100644
--- a/drivers/iio/adc/ad4030.c
+++ b/drivers/iio/adc/ad4030.c
@@ -233,9 +233,11 @@ struct ad4030_state {
}
static const int ad4030_average_modes[] = {
- 1, 2, 4, 8, 16, 32, 64, 128,
- 256, 512, 1024, 2048, 4096, 8192, 16384, 32768,
- 65536,
+ BIT(0), /* No averaging/oversampling */
+ BIT(1), BIT(2), BIT(3), BIT(4), /* 2 to 16 */
+ BIT(5), BIT(6), BIT(7), BIT(8), /* 32 to 256 */
+ BIT(9), BIT(10), BIT(11), BIT(12), /* 512 to 4096 */
+ BIT(13), BIT(14), BIT(15), BIT(16), /* 8192 to 65536 */
};
static int ad4030_enter_config_mode(struct ad4030_state *st)
--
2.39.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v3 6/8] iio: adc: ad4030: Add SPI offload support
2025-09-26 20:37 [PATCH v3 0/8] Add SPI offload support to AD4030 Marcelo Schmitt
` (4 preceding siblings ...)
2025-09-26 20:40 ` [PATCH v3 5/8] iio: adc: ad4030: Use BIT macro to improve code readability Marcelo Schmitt
@ 2025-09-26 20:40 ` Marcelo Schmitt
2025-09-27 12:59 ` kernel test robot
2025-09-28 10:08 ` Jonathan Cameron
2025-09-26 20:40 ` [PATCH v3 7/8] dt-bindings: iio: adc: adi,ad4030: Add ADAQ4216 and ADAQ4224 Marcelo Schmitt
2025-09-26 20:41 ` [PATCH v3 8/8] iio: adc: ad4030: Add support for " Marcelo Schmitt
7 siblings, 2 replies; 23+ messages in thread
From: Marcelo Schmitt @ 2025-09-26 20:40 UTC (permalink / raw)
To: linux-iio, devicetree, linux-doc, linux-spi, linux-kernel
Cc: jic23, michael.hennerich, nuno.sa, eblanc, dlechner, andy, robh,
krzk+dt, conor+dt, corbet, marcelo.schmitt1, Trevor Gamblin,
Axel Haslam
AD4030 and similar ADCs can capture data at sample rates up to 2 mega
samples per second (MSPS). Not all SPI controllers are able to achieve such
high throughputs and even when the controller is fast enough to run
transfers at the required speed, it may be costly to the CPU to handle
transfer data at such high sample rates. Add SPI offload support for AD4030
and similar ADCs to enable data capture at maximum sample rates.
Co-developed-by: Trevor Gamblin <tgamblin@baylibre.com>
Signed-off-by: Trevor Gamblin <tgamblin@baylibre.com>
Co-developed-by: Axel Haslam <ahaslam@baylibre.com>
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
Change log v2 -> v3
- Dropped Nuno's and Sergiu's co-developed-by tags as suggested by Nuno.
- No longer shadowing original error on ad4030_offload_buffer_postenable().
- Select SPI_OFFLOAD_TRIGGER_PWM instead of depending on PWM.
- Renamed __ad4030_set_sampling_freq() to ad4030_update_conversion_rate().
- Dropped st->mode check in ad4030_update_conversion_rate().
- Dropped ad4030_state mutex lock.
- Let SPI offload transfers run at the max speed specified in device tree.
- Don't round offload transfer bits_per_word to power of two.
- Take common-mode data size into account for offload xfer bits_per_word.
- Disallowed SPI offload for 2 channel interleaved mode.
drivers/iio/adc/Kconfig | 3 +
drivers/iio/adc/ad4030.c | 502 +++++++++++++++++++++++++++++++++++----
2 files changed, 463 insertions(+), 42 deletions(-)
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 58a14e6833f6..b1daded8bb87 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -62,7 +62,10 @@ config AD4030
depends on GPIOLIB
select REGMAP
select IIO_BUFFER
+ select IIO_BUFFER_DMA
+ select IIO_BUFFER_DMAENGINE
select IIO_TRIGGERED_BUFFER
+ select SPI_OFFLOAD_TRIGGER_PWM
help
Say yes here to build support for Analog Devices AD4030 and AD4630 high speed
SPI analog to digital converters (ADC).
diff --git a/drivers/iio/adc/ad4030.c b/drivers/iio/adc/ad4030.c
index cdf5933e9725..8fca98738e3e 100644
--- a/drivers/iio/adc/ad4030.c
+++ b/drivers/iio/adc/ad4030.c
@@ -14,15 +14,25 @@
*/
#include <linux/bitfield.h>
+#include <linux/cleanup.h>
#include <linux/clk.h>
+#include <linux/dmaengine.h>
+#include <linux/iio/buffer-dmaengine.h>
#include <linux/iio/iio.h>
#include <linux/iio/trigger_consumer.h>
#include <linux/iio/triggered_buffer.h>
+#include <linux/limits.h>
+#include <linux/log2.h>
+#include <linux/math64.h>
+#include <linux/minmax.h>
+#include <linux/pwm.h>
#include <linux/regmap.h>
#include <linux/regulator/consumer.h>
+#include <linux/spi/offload/consumer.h>
#include <linux/spi/spi.h>
#include <linux/unaligned.h>
#include <linux/units.h>
+#include <linux/types.h>
#define AD4030_REG_INTERFACE_CONFIG_A 0x00
#define AD4030_REG_INTERFACE_CONFIG_A_SW_RESET (BIT(0) | BIT(7))
@@ -111,6 +121,8 @@
#define AD4632_TCYC_NS 2000
#define AD4632_TCYC_ADJUSTED_NS (AD4632_TCYC_NS - AD4030_TCNVL_NS)
#define AD4030_TRESET_COM_DELAY_MS 750
+/* Datasheet says 9.8ns, so use the closest integer value */
+#define AD4030_TQUIET_CNV_DELAY_NS 10
enum ad4030_out_mode {
AD4030_OUT_DATA_MD_DIFF,
@@ -136,11 +148,13 @@ struct ad4030_chip_info {
const char *name;
const unsigned long *available_masks;
const struct iio_chan_spec channels[AD4030_MAX_IIO_CHANNEL_NB];
+ const struct iio_chan_spec offload_channels[AD4030_MAX_IIO_CHANNEL_NB];
u8 grade;
u8 precision_bits;
/* Number of hardware channels */
int num_voltage_inputs;
unsigned int tcyc_ns;
+ unsigned int max_sample_rate_hz;
};
struct ad4030_state {
@@ -153,6 +167,14 @@ struct ad4030_state {
int offset_avail[3];
unsigned int avg_log2;
enum ad4030_out_mode mode;
+ /* Offload sampling */
+ struct spi_transfer offload_xfer;
+ struct spi_message offload_msg;
+ struct spi_offload *offload;
+ struct spi_offload_trigger *offload_trigger;
+ struct spi_offload_trigger_config offload_trigger_config;
+ struct pwm_device *cnv_trigger;
+ struct pwm_waveform cnv_wf;
/*
* DMA (thus cache coherency maintenance) requires the transfer buffers
@@ -209,8 +231,9 @@ struct ad4030_state {
* - voltage0-voltage1
* - voltage2-voltage3
*/
-#define AD4030_CHAN_DIFF(_idx, _scan_type) { \
+#define __AD4030_CHAN_DIFF(_idx, _scan_type, _offload) { \
.info_mask_shared_by_all = \
+ (_offload ? BIT(IIO_CHAN_INFO_SAMP_FREQ) : 0) | \
BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
.info_mask_shared_by_all_available = \
BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
@@ -232,6 +255,12 @@ struct ad4030_state {
.num_ext_scan_type = ARRAY_SIZE(_scan_type), \
}
+#define AD4030_CHAN_DIFF(_idx, _scan_type) \
+ __AD4030_CHAN_DIFF(_idx, _scan_type, 0)
+
+#define AD4030_OFFLOAD_CHAN_DIFF(_idx, _scan_type) \
+ __AD4030_CHAN_DIFF(_idx, _scan_type, 1)
+
static const int ad4030_average_modes[] = {
BIT(0), /* No averaging/oversampling */
BIT(1), BIT(2), BIT(3), BIT(4), /* 2 to 16 */
@@ -240,6 +269,11 @@ static const int ad4030_average_modes[] = {
BIT(13), BIT(14), BIT(15), BIT(16), /* 8192 to 65536 */
};
+static const struct spi_offload_config ad4030_offload_config = {
+ .capability_flags = SPI_OFFLOAD_CAP_TRIGGER |
+ SPI_OFFLOAD_CAP_RX_STREAM_DMA,
+};
+
static int ad4030_enter_config_mode(struct ad4030_state *st)
{
st->tx_data[0] = AD4030_REG_ACCESS;
@@ -453,6 +487,107 @@ static int ad4030_get_chan_calibbias(struct iio_dev *indio_dev,
}
}
+static void ad4030_get_sampling_freq(struct ad4030_state *st, int *freq)
+{
+ struct spi_offload_trigger_config *config = &st->offload_trigger_config;
+
+ /*
+ * Conversion data is fetched from the device when the offload transfer
+ * is triggered. Thus, provide the SPI offload trigger frequency as the
+ * sampling frequency.
+ */
+ *freq = config->periodic.frequency_hz;
+}
+
+static int ad4030_update_conversion_rate(struct ad4030_state *st,
+ unsigned int freq, unsigned int avg_log2)
+{
+ struct spi_offload_trigger_config *config = &st->offload_trigger_config;
+ struct pwm_waveform cnv_wf = { };
+ u64 target = AD4030_TCNVH_NS;
+ u64 offload_period_ns;
+ u64 offload_offset_ns;
+ int ret;
+
+ /*
+ * When averaging/oversampling over N samples, we fire the offload
+ * trigger once at every N pulses of the CNV signal. Conversely, the CNV
+ * signal needs to be N times faster than the offload trigger. Take that
+ * into account to correctly re-evaluate both the PWM waveform connected
+ * to CNV and the SPI offload trigger.
+ */
+ freq <<= avg_log2;
+
+ cnv_wf.period_length_ns = DIV_ROUND_CLOSEST(NSEC_PER_SEC, freq);
+ /*
+ * The datasheet lists a minimum time of 9.8 ns, but no maximum. If the
+ * rounded PWM's value is less than 10, increase the target value by 10
+ * and attempt to round the waveform again, until the value is at least
+ * 10 ns. Use a separate variable to represent the target in case the
+ * rounding is severe enough to keep putting the first few results under
+ * the minimum 10ns condition checked by the while loop.
+ */
+ do {
+ cnv_wf.duty_length_ns = target;
+ ret = pwm_round_waveform_might_sleep(st->cnv_trigger, &cnv_wf);
+ if (ret)
+ return ret;
+ target += AD4030_TCNVH_NS;
+ } while (cnv_wf.duty_length_ns < AD4030_TCNVH_NS);
+
+ if (!in_range(cnv_wf.period_length_ns, AD4030_TCYC_NS, INT_MAX))
+ return -EINVAL;
+
+ offload_period_ns = cnv_wf.period_length_ns;
+ /*
+ * Make the offload trigger period be N times longer than the CNV PWM
+ * period when averaging over N samples.
+ */
+ offload_period_ns <<= avg_log2;
+
+ config->periodic.frequency_hz = DIV_ROUND_UP_ULL(NSEC_PER_SEC,
+ offload_period_ns);
+
+ /*
+ * The hardware does the capture on zone 2 (when SPI trigger PWM
+ * is used). This means that the SPI trigger signal should happen at
+ * tsync + tquiet_con_delay being tsync the conversion signal period
+ * and tquiet_con_delay 9.8ns. Hence set the PWM phase accordingly.
+ *
+ * The PWM waveform API only supports nanosecond resolution right now,
+ * so round this setting to the closest available value.
+ */
+ offload_offset_ns = AD4030_TQUIET_CNV_DELAY_NS;
+ do {
+ config->periodic.offset_ns = offload_offset_ns;
+ ret = spi_offload_trigger_validate(st->offload_trigger, config);
+ if (ret)
+ return ret;
+ offload_offset_ns += AD4030_TQUIET_CNV_DELAY_NS;
+ } while (config->periodic.offset_ns < AD4030_TQUIET_CNV_DELAY_NS);
+
+ st->cnv_wf = cnv_wf;
+
+ return 0;
+}
+
+static int ad4030_set_sampling_freq(struct iio_dev *indio_dev, int freq)
+{
+ struct ad4030_state *st = iio_priv(indio_dev);
+
+ /*
+ * We have no control over the sampling frequency without SPI offload
+ * triggering.
+ */
+ if (!st->offload_trigger)
+ return -ENODEV;
+
+ if (!in_range(freq, 1, st->chip->max_sample_rate_hz))
+ return -EINVAL;
+
+ return ad4030_update_conversion_rate(st, freq, st->avg_log2);
+}
+
static int ad4030_set_chan_calibscale(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
int gain_int,
@@ -507,27 +642,6 @@ static int ad4030_set_chan_calibbias(struct iio_dev *indio_dev,
st->tx_data, AD4030_REG_OFFSET_BYTES_NB);
}
-static int ad4030_set_avg_frame_len(struct iio_dev *dev, int avg_val)
-{
- struct ad4030_state *st = iio_priv(dev);
- unsigned int avg_log2 = ilog2(avg_val);
- unsigned int last_avg_idx = ARRAY_SIZE(ad4030_average_modes) - 1;
- int ret;
-
- if (avg_val < 0 || avg_val > ad4030_average_modes[last_avg_idx])
- return -EINVAL;
-
- ret = regmap_write(st->regmap, AD4030_REG_AVG,
- AD4030_REG_AVG_MASK_AVG_SYNC |
- FIELD_PREP(AD4030_REG_AVG_MASK_AVG_VAL, avg_log2));
- if (ret)
- return ret;
-
- st->avg_log2 = avg_log2;
-
- return 0;
-}
-
static bool ad4030_is_common_byte_asked(struct ad4030_state *st,
unsigned int mask)
{
@@ -536,11 +650,10 @@ static bool ad4030_is_common_byte_asked(struct ad4030_state *st,
AD4030_DUAL_COMMON_BYTE_CHANNELS_MASK);
}
-static int ad4030_set_mode(struct iio_dev *indio_dev, unsigned long mask)
+static int ad4030_set_mode(struct ad4030_state *st, unsigned long mask,
+ unsigned int avg_log2)
{
- struct ad4030_state *st = iio_priv(indio_dev);
-
- if (st->avg_log2 > 0) {
+ if (avg_log2 > 0) {
st->mode = AD4030_OUT_DATA_MD_30_AVERAGED_DIFF;
} else if (ad4030_is_common_byte_asked(st, mask)) {
switch (st->chip->precision_bits) {
@@ -564,6 +677,49 @@ static int ad4030_set_mode(struct iio_dev *indio_dev, unsigned long mask)
st->mode);
}
+static int ad4030_set_avg_frame_len(struct iio_dev *dev, unsigned long mask, int avg_val)
+{
+ struct ad4030_state *st = iio_priv(dev);
+ unsigned int avg_log2 = ilog2(avg_val);
+ unsigned int last_avg_idx = ARRAY_SIZE(ad4030_average_modes) - 1;
+ int freq;
+ int ret;
+
+ if (avg_val < 0 || avg_val > ad4030_average_modes[last_avg_idx])
+ return -EINVAL;
+
+ ret = ad4030_set_mode(st, mask, avg_log2);
+ if (ret)
+ return ret;
+
+ if (st->offload_trigger) {
+ /*
+ * The sample averaging and sampling frequency configurations
+ * are mutually dependent one from another. That's because the
+ * effective data sample rate is fCNV / 2^N, where N is the
+ * number of samples being averaged.
+ *
+ * When SPI offload is supported and we have control over the
+ * sample rate, the conversion start signal (CNV) and the SPI
+ * offload trigger frequencies must be re-evaluated so data is
+ * fetched only after 'avg_val' conversions.
+ */
+ ad4030_get_sampling_freq(st, &freq);
+ ret = ad4030_update_conversion_rate(st, freq, avg_log2);
+ if (ret)
+ return ret;
+ }
+
+ ret = regmap_write(st->regmap, AD4030_REG_AVG,
+ AD4030_REG_AVG_MASK_AVG_SYNC |
+ FIELD_PREP(AD4030_REG_AVG_MASK_AVG_VAL, avg_log2));
+ if (ret)
+ return ret;
+
+ st->avg_log2 = avg_log2;
+ return 0;
+}
+
/*
* Descramble 2 32bits numbers out of a 64bits. The bits are interleaved:
* 1 bit for first number, 1 bit for the second, and so on...
@@ -672,7 +828,7 @@ static int ad4030_single_conversion(struct iio_dev *indio_dev,
struct ad4030_state *st = iio_priv(indio_dev);
int ret;
- ret = ad4030_set_mode(indio_dev, BIT(chan->scan_index));
+ ret = ad4030_set_mode(st, BIT(chan->scan_index), st->avg_log2);
if (ret)
return ret;
@@ -769,6 +925,13 @@ static int ad4030_read_raw_dispatch(struct iio_dev *indio_dev,
*val = BIT(st->avg_log2);
return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ if (!st->offload_trigger)
+ return -ENODEV;
+
+ ad4030_get_sampling_freq(st, val);
+ return IIO_VAL_INT;
+
default:
return -EINVAL;
}
@@ -807,7 +970,10 @@ static int ad4030_write_raw_dispatch(struct iio_dev *indio_dev,
return ad4030_set_chan_calibbias(indio_dev, chan, val);
case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
- return ad4030_set_avg_frame_len(indio_dev, val);
+ return ad4030_set_avg_frame_len(indio_dev, BIT(chan->scan_index), val);
+
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ return ad4030_set_sampling_freq(indio_dev, val);
default:
return -EINVAL;
@@ -869,7 +1035,9 @@ static int ad4030_get_current_scan_type(const struct iio_dev *indio_dev,
static int ad4030_update_scan_mode(struct iio_dev *indio_dev,
const unsigned long *scan_mask)
{
- return ad4030_set_mode(indio_dev, *scan_mask);
+ struct ad4030_state *st = iio_priv(indio_dev);
+
+ return ad4030_set_mode(st, *scan_mask, st->avg_log2);
}
static const struct iio_info ad4030_iio_info = {
@@ -898,6 +1066,108 @@ static const struct iio_buffer_setup_ops ad4030_buffer_setup_ops = {
.validate_scan_mask = ad4030_validate_scan_mask,
};
+static void ad4030_prepare_offload_msg(struct iio_dev *indio_dev)
+{
+ struct ad4030_state *st = iio_priv(indio_dev);
+ u8 offload_bpw;
+
+ if (st->mode == AD4030_OUT_DATA_MD_30_AVERAGED_DIFF) {
+ offload_bpw = 32;
+ } else {
+ offload_bpw = st->chip->precision_bits;
+ offload_bpw += (st->mode == AD4030_OUT_DATA_MD_24_DIFF_8_COM ||
+ st->mode == AD4030_OUT_DATA_MD_16_DIFF_8_COM) ? 8 : 0;
+ }
+
+ st->offload_xfer.bits_per_word = offload_bpw;
+ st->offload_xfer.len = spi_bpw_to_bytes(offload_bpw);
+ st->offload_xfer.offload_flags = SPI_OFFLOAD_XFER_RX_STREAM;
+ spi_message_init_with_transfers(&st->offload_msg, &st->offload_xfer, 1);
+}
+
+static int ad4030_offload_buffer_postenable(struct iio_dev *indio_dev)
+{
+ struct ad4030_state *st = iio_priv(indio_dev);
+ unsigned int reg_modes;
+ int ret, ret2;
+
+ /*
+ * When data from 2 analog input channels is output through a single
+ * bus line (interleaved mode (LANE_MD == 0b11)) and gets pushed through
+ * DMA, extra hardware is required to do the de-interleaving. While we
+ * don't support such hardware configurations, disallow interleaved mode
+ * when using SPI offload.
+ */
+ ret = regmap_read(st->regmap, AD4030_REG_MODES, ®_modes);
+ if (ret)
+ return ret;
+
+ if (st->chip->num_voltage_inputs > 1 &&
+ FIELD_GET(AD4030_REG_MODES_MASK_LANE_MODE, reg_modes) == AD4030_LANE_MD_INTERLEAVED)
+ return -EINVAL;
+
+ ret = regmap_write(st->regmap, AD4030_REG_EXIT_CFG_MODE, BIT(0));
+ if (ret)
+ return ret;
+
+ ad4030_prepare_offload_msg(indio_dev);
+ st->offload_msg.offload = st->offload;
+ ret = spi_optimize_message(st->spi, &st->offload_msg);
+ if (ret)
+ goto out_reset_mode;
+
+ ret = pwm_set_waveform_might_sleep(st->cnv_trigger, &st->cnv_wf, false);
+ if (ret)
+ goto out_unoptimize;
+
+ ret = spi_offload_trigger_enable(st->offload, st->offload_trigger,
+ &st->offload_trigger_config);
+ if (ret)
+ goto out_pwm_disable;
+
+ return 0;
+
+out_pwm_disable:
+ pwm_disable(st->cnv_trigger);
+out_unoptimize:
+ spi_unoptimize_message(&st->offload_msg);
+out_reset_mode:
+ /* reenter register configuration mode */
+ ret2 = ad4030_enter_config_mode(st);
+ if (ret2)
+ dev_err(&st->spi->dev,
+ "couldn't reenter register configuration mode: %d\n",
+ ret2);
+
+ return ret;
+}
+
+static int ad4030_offload_buffer_predisable(struct iio_dev *indio_dev)
+{
+ struct ad4030_state *st = iio_priv(indio_dev);
+ int ret;
+
+ spi_offload_trigger_disable(st->offload, st->offload_trigger);
+
+ pwm_disable(st->cnv_trigger);
+
+ spi_unoptimize_message(&st->offload_msg);
+
+ /* reenter register configuration mode */
+ ret = ad4030_enter_config_mode(st);
+ if (ret)
+ dev_err(&st->spi->dev,
+ "couldn't reenter register configuration mode\n");
+
+ return ret;
+}
+
+static const struct iio_buffer_setup_ops ad4030_offload_buffer_setup_ops = {
+ .postenable = &ad4030_offload_buffer_postenable,
+ .predisable = &ad4030_offload_buffer_predisable,
+ .validate_scan_mask = ad4030_validate_scan_mask,
+};
+
static int ad4030_regulators_get(struct ad4030_state *st)
{
struct device *dev = &st->spi->dev;
@@ -967,6 +1237,24 @@ static int ad4030_detect_chip_info(const struct ad4030_state *st)
return 0;
}
+static int ad4030_pwm_get(struct ad4030_state *st)
+{
+ struct device *dev = &st->spi->dev;
+
+ st->cnv_trigger = devm_pwm_get(dev, NULL);
+ if (IS_ERR(st->cnv_trigger))
+ return dev_err_probe(dev, PTR_ERR(st->cnv_trigger),
+ "Failed to get CNV PWM\n");
+
+ /*
+ * Preemptively disable the PWM, since we only want to enable it with
+ * the buffer.
+ */
+ pwm_disable(st->cnv_trigger);
+
+ return 0;
+}
+
static int ad4030_config(struct ad4030_state *st)
{
int ret;
@@ -994,6 +1282,31 @@ static int ad4030_config(struct ad4030_state *st)
return 0;
}
+static int ad4030_spi_offload_setup(struct iio_dev *indio_dev,
+ struct ad4030_state *st)
+{
+ struct device *dev = &st->spi->dev;
+ struct dma_chan *rx_dma;
+
+ indio_dev->setup_ops = &ad4030_offload_buffer_setup_ops;
+
+ st->offload_trigger = devm_spi_offload_trigger_get(dev, st->offload,
+ SPI_OFFLOAD_TRIGGER_PERIODIC);
+ if (IS_ERR(st->offload_trigger))
+ return dev_err_probe(dev, PTR_ERR(st->offload_trigger),
+ "failed to get offload trigger\n");
+
+ st->offload_trigger_config.type = SPI_OFFLOAD_TRIGGER_PERIODIC;
+
+ rx_dma = devm_spi_offload_rx_stream_request_dma_chan(dev, st->offload);
+ if (IS_ERR(rx_dma))
+ return dev_err_probe(dev, PTR_ERR(rx_dma),
+ "failed to get offload RX DMA\n");
+
+ return devm_iio_dmaengine_buffer_setup_with_handle(dev, indio_dev, rx_dma,
+ IIO_BUFFER_DIRECTION_IN);
+}
+
static int ad4030_probe(struct spi_device *spi)
{
struct device *dev = &spi->dev;
@@ -1045,24 +1358,61 @@ static int ad4030_probe(struct spi_device *spi)
return dev_err_probe(dev, PTR_ERR(st->cnv_gpio),
"Failed to get cnv gpio\n");
- /*
- * One hardware channel is split in two software channels when using
- * common byte mode. Add one more channel for the timestamp.
- */
- indio_dev->num_channels = 2 * st->chip->num_voltage_inputs + 1;
indio_dev->name = st->chip->name;
indio_dev->modes = INDIO_DIRECT_MODE;
indio_dev->info = &ad4030_iio_info;
- indio_dev->channels = st->chip->channels;
indio_dev->available_scan_masks = st->chip->available_masks;
- ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
- iio_pollfunc_store_time,
- ad4030_trigger_handler,
- &ad4030_buffer_setup_ops);
- if (ret)
- return dev_err_probe(dev, ret,
- "Failed to setup triggered buffer\n");
+ st->offload = devm_spi_offload_get(dev, spi, &ad4030_offload_config);
+ ret = PTR_ERR_OR_ZERO(st->offload);
+ if (ret && ret != -ENODEV)
+ return dev_err_probe(dev, ret, "failed to get offload\n");
+
+ /* Fall back to low speed usage when no SPI offload is available. */
+ if (ret == -ENODEV) {
+ /*
+ * One hardware channel is split in two software channels when
+ * using common byte mode. Add one more channel for the timestamp.
+ */
+ indio_dev->num_channels = 2 * st->chip->num_voltage_inputs + 1;
+ indio_dev->channels = st->chip->channels;
+
+ ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
+ iio_pollfunc_store_time,
+ ad4030_trigger_handler,
+ &ad4030_buffer_setup_ops);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to setup triggered buffer\n");
+ } else {
+ /*
+ * One hardware channel is split in two software channels when
+ * using common byte mode. Offloaded SPI transfers can't support
+ * software timestamp so no additional timestamp channel is added.
+ */
+ indio_dev->num_channels = 2 * st->chip->num_voltage_inputs;
+ indio_dev->channels = st->chip->offload_channels;
+ ret = ad4030_spi_offload_setup(indio_dev, st);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to setup SPI offload\n");
+
+ ret = ad4030_pwm_get(st);
+ if (ret)
+ return dev_err_probe(&spi->dev, ret,
+ "Failed to get PWM: %d\n", ret);
+
+ /*
+ * Start with a slower sampling rate so there is some room for
+ * adjusting the sample averaging and the sampling frequency
+ * without hitting the maximum conversion rate.
+ */
+ ret = ad4030_update_conversion_rate(st, st->chip->max_sample_rate_hz >> 4,
+ st->avg_log2);
+ if (ret)
+ return dev_err_probe(&spi->dev, ret,
+ "Failed to set offload samp freq\n");
+ }
return devm_iio_device_register(dev, indio_dev);
}
@@ -1100,6 +1450,23 @@ static const struct iio_scan_type ad4030_24_scan_types[] = {
},
};
+static const struct iio_scan_type ad4030_24_offload_scan_types[] = {
+ [AD4030_SCAN_TYPE_NORMAL] = {
+ .sign = 's',
+ .storagebits = 32,
+ .realbits = 24,
+ .shift = 0,
+ .endianness = IIO_CPU,
+ },
+ [AD4030_SCAN_TYPE_AVG] = {
+ .sign = 's',
+ .storagebits = 32,
+ .realbits = 30,
+ .shift = 2,
+ .endianness = IIO_CPU,
+ },
+};
+
static const struct iio_scan_type ad4030_16_scan_types[] = {
[AD4030_SCAN_TYPE_NORMAL] = {
.sign = 's',
@@ -1117,6 +1484,23 @@ static const struct iio_scan_type ad4030_16_scan_types[] = {
}
};
+static const struct iio_scan_type ad4030_16_offload_scan_types[] = {
+ [AD4030_SCAN_TYPE_NORMAL] = {
+ .sign = 's',
+ .storagebits = 32,
+ .realbits = 16,
+ .shift = 0,
+ .endianness = IIO_CPU,
+ },
+ [AD4030_SCAN_TYPE_AVG] = {
+ .sign = 's',
+ .storagebits = 32,
+ .realbits = 30,
+ .shift = 2,
+ .endianness = IIO_CPU,
+ },
+};
+
static const struct ad4030_chip_info ad4030_24_chip_info = {
.name = "ad4030-24",
.available_masks = ad4030_channel_masks,
@@ -1125,10 +1509,15 @@ static const struct ad4030_chip_info ad4030_24_chip_info = {
AD4030_CHAN_CMO(1, 0),
IIO_CHAN_SOFT_TIMESTAMP(2),
},
+ .offload_channels = {
+ AD4030_OFFLOAD_CHAN_DIFF(0, ad4030_24_offload_scan_types),
+ AD4030_CHAN_CMO(1, 0),
+ },
.grade = AD4030_REG_CHIP_GRADE_AD4030_24_GRADE,
.precision_bits = 24,
.num_voltage_inputs = 1,
.tcyc_ns = AD4030_TCYC_ADJUSTED_NS,
+ .max_sample_rate_hz = 2 * HZ_PER_MHZ,
};
static const struct ad4030_chip_info ad4630_16_chip_info = {
@@ -1141,10 +1530,17 @@ static const struct ad4030_chip_info ad4630_16_chip_info = {
AD4030_CHAN_CMO(3, 1),
IIO_CHAN_SOFT_TIMESTAMP(4),
},
+ .offload_channels = {
+ AD4030_OFFLOAD_CHAN_DIFF(0, ad4030_16_offload_scan_types),
+ AD4030_OFFLOAD_CHAN_DIFF(1, ad4030_16_offload_scan_types),
+ AD4030_CHAN_CMO(2, 0),
+ AD4030_CHAN_CMO(3, 1),
+ },
.grade = AD4030_REG_CHIP_GRADE_AD4630_16_GRADE,
.precision_bits = 16,
.num_voltage_inputs = 2,
.tcyc_ns = AD4030_TCYC_ADJUSTED_NS,
+ .max_sample_rate_hz = 2 * HZ_PER_MHZ,
};
static const struct ad4030_chip_info ad4630_24_chip_info = {
@@ -1157,10 +1553,17 @@ static const struct ad4030_chip_info ad4630_24_chip_info = {
AD4030_CHAN_CMO(3, 1),
IIO_CHAN_SOFT_TIMESTAMP(4),
},
+ .offload_channels = {
+ AD4030_OFFLOAD_CHAN_DIFF(0, ad4030_24_offload_scan_types),
+ AD4030_OFFLOAD_CHAN_DIFF(1, ad4030_24_offload_scan_types),
+ AD4030_CHAN_CMO(2, 0),
+ AD4030_CHAN_CMO(3, 1),
+ },
.grade = AD4030_REG_CHIP_GRADE_AD4630_24_GRADE,
.precision_bits = 24,
.num_voltage_inputs = 2,
.tcyc_ns = AD4030_TCYC_ADJUSTED_NS,
+ .max_sample_rate_hz = 2 * HZ_PER_MHZ,
};
static const struct ad4030_chip_info ad4632_16_chip_info = {
@@ -1173,10 +1576,17 @@ static const struct ad4030_chip_info ad4632_16_chip_info = {
AD4030_CHAN_CMO(3, 1),
IIO_CHAN_SOFT_TIMESTAMP(4),
},
+ .offload_channels = {
+ AD4030_OFFLOAD_CHAN_DIFF(0, ad4030_16_offload_scan_types),
+ AD4030_OFFLOAD_CHAN_DIFF(1, ad4030_16_offload_scan_types),
+ AD4030_CHAN_CMO(2, 0),
+ AD4030_CHAN_CMO(3, 1),
+ },
.grade = AD4030_REG_CHIP_GRADE_AD4632_16_GRADE,
.precision_bits = 16,
.num_voltage_inputs = 2,
.tcyc_ns = AD4632_TCYC_ADJUSTED_NS,
+ .max_sample_rate_hz = 500 * HZ_PER_KHZ,
};
static const struct ad4030_chip_info ad4632_24_chip_info = {
@@ -1189,10 +1599,17 @@ static const struct ad4030_chip_info ad4632_24_chip_info = {
AD4030_CHAN_CMO(3, 1),
IIO_CHAN_SOFT_TIMESTAMP(4),
},
+ .offload_channels = {
+ AD4030_OFFLOAD_CHAN_DIFF(0, ad4030_24_offload_scan_types),
+ AD4030_OFFLOAD_CHAN_DIFF(1, ad4030_24_offload_scan_types),
+ AD4030_CHAN_CMO(2, 0),
+ AD4030_CHAN_CMO(3, 1),
+ },
.grade = AD4030_REG_CHIP_GRADE_AD4632_24_GRADE,
.precision_bits = 24,
.num_voltage_inputs = 2,
.tcyc_ns = AD4632_TCYC_ADJUSTED_NS,
+ .max_sample_rate_hz = 500 * HZ_PER_KHZ,
};
static const struct spi_device_id ad4030_id_table[] = {
@@ -1228,3 +1645,4 @@ module_spi_driver(ad4030_driver);
MODULE_AUTHOR("Esteban Blanc <eblanc@baylibre.com>");
MODULE_DESCRIPTION("Analog Devices AD4630 ADC family driver");
MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("IIO_DMAENGINE_BUFFER");
--
2.39.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v3 6/8] iio: adc: ad4030: Add SPI offload support
2025-09-26 20:40 ` [PATCH v3 6/8] iio: adc: ad4030: Add SPI offload support Marcelo Schmitt
@ 2025-09-27 12:59 ` kernel test robot
2025-09-28 10:02 ` Jonathan Cameron
2025-09-28 10:08 ` Jonathan Cameron
1 sibling, 1 reply; 23+ messages in thread
From: kernel test robot @ 2025-09-27 12:59 UTC (permalink / raw)
To: Marcelo Schmitt, linux-iio, devicetree, linux-doc, linux-spi,
linux-kernel
Cc: oe-kbuild-all, jic23, michael.hennerich, nuno.sa, eblanc,
dlechner, andy, robh, krzk+dt, conor+dt, corbet, marcelo.schmitt1,
Trevor Gamblin, Axel Haslam
Hi Marcelo,
kernel test robot noticed the following build errors:
[auto build test ERROR on 561285d048053fec8a3d6d1e3ddc60df11c393a0]
url: https://github.com/intel-lab-lkp/linux/commits/Marcelo-Schmitt/dt-bindings-iio-adc-adi-ad4030-Reference-spi-peripheral-props/20250927-044546
base: 561285d048053fec8a3d6d1e3ddc60df11c393a0
patch link: https://lore.kernel.org/r/0028720d2cb21898ef044458065ac8a0bc829588.1758916484.git.marcelo.schmitt%40analog.com
patch subject: [PATCH v3 6/8] iio: adc: ad4030: Add SPI offload support
config: i386-randconfig-014-20250927 (https://download.01.org/0day-ci/archive/20250927/202509272028.0zLNiR5w-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250927/202509272028.0zLNiR5w-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509272028.0zLNiR5w-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/spi/spi-offload-trigger-pwm.c: In function 'spi_offload_trigger_pwm_validate':
>> drivers/spi/spi-offload-trigger-pwm.c:55:15: error: implicit declaration of function 'pwm_round_waveform_might_sleep' [-Wimplicit-function-declaration]
55 | ret = pwm_round_waveform_might_sleep(st->pwm, &wf);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/spi/spi-offload-trigger-pwm.c: In function 'spi_offload_trigger_pwm_enable':
>> drivers/spi/spi-offload-trigger-pwm.c:81:16: error: implicit declaration of function 'pwm_set_waveform_might_sleep' [-Wimplicit-function-declaration]
81 | return pwm_set_waveform_might_sleep(st->pwm, &wf, false);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/spi/spi-offload-trigger-pwm.c: In function 'spi_offload_trigger_pwm_disable':
>> drivers/spi/spi-offload-trigger-pwm.c:90:15: error: implicit declaration of function 'pwm_get_waveform_might_sleep' [-Wimplicit-function-declaration]
90 | ret = pwm_get_waveform_might_sleep(st->pwm, &wf);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for SPI_OFFLOAD_TRIGGER_PWM
Depends on [n]: SPI [=y] && SPI_OFFLOAD [=y] && PWM [=n]
Selected by [y]:
- AD4030 [=y] && IIO [=y] && SPI [=y] && GPIOLIB [=y]
vim +/pwm_round_waveform_might_sleep +55 drivers/spi/spi-offload-trigger-pwm.c
ebb398ae1e052c David Lechner 2025-02-07 36
ebb398ae1e052c David Lechner 2025-02-07 37 static int spi_offload_trigger_pwm_validate(struct spi_offload_trigger *trigger,
ebb398ae1e052c David Lechner 2025-02-07 38 struct spi_offload_trigger_config *config)
ebb398ae1e052c David Lechner 2025-02-07 39 {
ebb398ae1e052c David Lechner 2025-02-07 40 struct spi_offload_trigger_pwm_state *st = spi_offload_trigger_get_priv(trigger);
ebb398ae1e052c David Lechner 2025-02-07 41 struct spi_offload_trigger_periodic *periodic = &config->periodic;
ebb398ae1e052c David Lechner 2025-02-07 42 struct pwm_waveform wf = { };
ebb398ae1e052c David Lechner 2025-02-07 43 int ret;
ebb398ae1e052c David Lechner 2025-02-07 44
ebb398ae1e052c David Lechner 2025-02-07 45 if (config->type != SPI_OFFLOAD_TRIGGER_PERIODIC)
ebb398ae1e052c David Lechner 2025-02-07 46 return -EINVAL;
ebb398ae1e052c David Lechner 2025-02-07 47
ebb398ae1e052c David Lechner 2025-02-07 48 if (!periodic->frequency_hz)
ebb398ae1e052c David Lechner 2025-02-07 49 return -EINVAL;
ebb398ae1e052c David Lechner 2025-02-07 50
ebb398ae1e052c David Lechner 2025-02-07 51 wf.period_length_ns = DIV_ROUND_UP_ULL(NSEC_PER_SEC, periodic->frequency_hz);
ebb398ae1e052c David Lechner 2025-02-07 52 /* REVISIT: 50% duty-cycle for now - may add config parameter later */
ebb398ae1e052c David Lechner 2025-02-07 53 wf.duty_length_ns = wf.period_length_ns / 2;
ebb398ae1e052c David Lechner 2025-02-07 54
ebb398ae1e052c David Lechner 2025-02-07 @55 ret = pwm_round_waveform_might_sleep(st->pwm, &wf);
ebb398ae1e052c David Lechner 2025-02-07 56 if (ret < 0)
ebb398ae1e052c David Lechner 2025-02-07 57 return ret;
ebb398ae1e052c David Lechner 2025-02-07 58
ebb398ae1e052c David Lechner 2025-02-07 59 periodic->frequency_hz = DIV_ROUND_UP_ULL(NSEC_PER_SEC, wf.period_length_ns);
ebb398ae1e052c David Lechner 2025-02-07 60
ebb398ae1e052c David Lechner 2025-02-07 61 return 0;
ebb398ae1e052c David Lechner 2025-02-07 62 }
ebb398ae1e052c David Lechner 2025-02-07 63
ebb398ae1e052c David Lechner 2025-02-07 64 static int spi_offload_trigger_pwm_enable(struct spi_offload_trigger *trigger,
ebb398ae1e052c David Lechner 2025-02-07 65 struct spi_offload_trigger_config *config)
ebb398ae1e052c David Lechner 2025-02-07 66 {
ebb398ae1e052c David Lechner 2025-02-07 67 struct spi_offload_trigger_pwm_state *st = spi_offload_trigger_get_priv(trigger);
ebb398ae1e052c David Lechner 2025-02-07 68 struct spi_offload_trigger_periodic *periodic = &config->periodic;
ebb398ae1e052c David Lechner 2025-02-07 69 struct pwm_waveform wf = { };
ebb398ae1e052c David Lechner 2025-02-07 70
ebb398ae1e052c David Lechner 2025-02-07 71 if (config->type != SPI_OFFLOAD_TRIGGER_PERIODIC)
ebb398ae1e052c David Lechner 2025-02-07 72 return -EINVAL;
ebb398ae1e052c David Lechner 2025-02-07 73
ebb398ae1e052c David Lechner 2025-02-07 74 if (!periodic->frequency_hz)
ebb398ae1e052c David Lechner 2025-02-07 75 return -EINVAL;
ebb398ae1e052c David Lechner 2025-02-07 76
ebb398ae1e052c David Lechner 2025-02-07 77 wf.period_length_ns = DIV_ROUND_UP_ULL(NSEC_PER_SEC, periodic->frequency_hz);
ebb398ae1e052c David Lechner 2025-02-07 78 /* REVISIT: 50% duty-cycle for now - may add config parameter later */
ebb398ae1e052c David Lechner 2025-02-07 79 wf.duty_length_ns = wf.period_length_ns / 2;
ebb398ae1e052c David Lechner 2025-02-07 80
ebb398ae1e052c David Lechner 2025-02-07 @81 return pwm_set_waveform_might_sleep(st->pwm, &wf, false);
ebb398ae1e052c David Lechner 2025-02-07 82 }
ebb398ae1e052c David Lechner 2025-02-07 83
ebb398ae1e052c David Lechner 2025-02-07 84 static void spi_offload_trigger_pwm_disable(struct spi_offload_trigger *trigger)
ebb398ae1e052c David Lechner 2025-02-07 85 {
ebb398ae1e052c David Lechner 2025-02-07 86 struct spi_offload_trigger_pwm_state *st = spi_offload_trigger_get_priv(trigger);
ebb398ae1e052c David Lechner 2025-02-07 87 struct pwm_waveform wf;
ebb398ae1e052c David Lechner 2025-02-07 88 int ret;
ebb398ae1e052c David Lechner 2025-02-07 89
ebb398ae1e052c David Lechner 2025-02-07 @90 ret = pwm_get_waveform_might_sleep(st->pwm, &wf);
ebb398ae1e052c David Lechner 2025-02-07 91 if (ret < 0) {
ebb398ae1e052c David Lechner 2025-02-07 92 dev_err(st->dev, "failed to get waveform: %d\n", ret);
ebb398ae1e052c David Lechner 2025-02-07 93 return;
ebb398ae1e052c David Lechner 2025-02-07 94 }
ebb398ae1e052c David Lechner 2025-02-07 95
ebb398ae1e052c David Lechner 2025-02-07 96 wf.duty_length_ns = 0;
ebb398ae1e052c David Lechner 2025-02-07 97
ebb398ae1e052c David Lechner 2025-02-07 98 ret = pwm_set_waveform_might_sleep(st->pwm, &wf, false);
ebb398ae1e052c David Lechner 2025-02-07 99 if (ret < 0)
ebb398ae1e052c David Lechner 2025-02-07 100 dev_err(st->dev, "failed to disable PWM: %d\n", ret);
ebb398ae1e052c David Lechner 2025-02-07 101 }
ebb398ae1e052c David Lechner 2025-02-07 102
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v3 6/8] iio: adc: ad4030: Add SPI offload support
2025-09-27 12:59 ` kernel test robot
@ 2025-09-28 10:02 ` Jonathan Cameron
0 siblings, 0 replies; 23+ messages in thread
From: Jonathan Cameron @ 2025-09-28 10:02 UTC (permalink / raw)
To: kernel test robot
Cc: Marcelo Schmitt, linux-iio, devicetree, linux-doc, linux-spi,
linux-kernel, oe-kbuild-all, michael.hennerich, nuno.sa, eblanc,
dlechner, andy, robh, krzk+dt, conor+dt, corbet, marcelo.schmitt1,
Trevor Gamblin, Axel Haslam, Uwe Kleine-König
On Sat, 27 Sep 2025 20:59:36 +0800
kernel test robot <lkp@intel.com> wrote:
> Hi Marcelo,
>
> kernel test robot noticed the following build errors:
So, question is stubs or add a dependency.
(Assuming there isn't a patch in flight already to add the stubs).
Uwe, does it make sense to add stubs for these, similar to many of the
other consumer interfaces?
Jonathan
>
> [auto build test ERROR on 561285d048053fec8a3d6d1e3ddc60df11c393a0]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Marcelo-Schmitt/dt-bindings-iio-adc-adi-ad4030-Reference-spi-peripheral-props/20250927-044546
> base: 561285d048053fec8a3d6d1e3ddc60df11c393a0
> patch link: https://lore.kernel.org/r/0028720d2cb21898ef044458065ac8a0bc829588.1758916484.git.marcelo.schmitt%40analog.com
> patch subject: [PATCH v3 6/8] iio: adc: ad4030: Add SPI offload support
> config: i386-randconfig-014-20250927 (https://download.01.org/0day-ci/archive/20250927/202509272028.0zLNiR5w-lkp@intel.com/config)
> compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250927/202509272028.0zLNiR5w-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202509272028.0zLNiR5w-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
> drivers/spi/spi-offload-trigger-pwm.c: In function 'spi_offload_trigger_pwm_validate':
> >> drivers/spi/spi-offload-trigger-pwm.c:55:15: error: implicit declaration of function 'pwm_round_waveform_might_sleep' [-Wimplicit-function-declaration]
> 55 | ret = pwm_round_waveform_might_sleep(st->pwm, &wf);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/spi/spi-offload-trigger-pwm.c: In function 'spi_offload_trigger_pwm_enable':
> >> drivers/spi/spi-offload-trigger-pwm.c:81:16: error: implicit declaration of function 'pwm_set_waveform_might_sleep' [-Wimplicit-function-declaration]
> 81 | return pwm_set_waveform_might_sleep(st->pwm, &wf, false);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/spi/spi-offload-trigger-pwm.c: In function 'spi_offload_trigger_pwm_disable':
> >> drivers/spi/spi-offload-trigger-pwm.c:90:15: error: implicit declaration of function 'pwm_get_waveform_might_sleep' [-Wimplicit-function-declaration]
> 90 | ret = pwm_get_waveform_might_sleep(st->pwm, &wf);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Kconfig warnings: (for reference only)
> WARNING: unmet direct dependencies detected for SPI_OFFLOAD_TRIGGER_PWM
> Depends on [n]: SPI [=y] && SPI_OFFLOAD [=y] && PWM [=n]
> Selected by [y]:
> - AD4030 [=y] && IIO [=y] && SPI [=y] && GPIOLIB [=y]
>
>
> vim +/pwm_round_waveform_might_sleep +55 drivers/spi/spi-offload-trigger-pwm.c
>
> ebb398ae1e052c David Lechner 2025-02-07 36
> ebb398ae1e052c David Lechner 2025-02-07 37 static int spi_offload_trigger_pwm_validate(struct spi_offload_trigger *trigger,
> ebb398ae1e052c David Lechner 2025-02-07 38 struct spi_offload_trigger_config *config)
> ebb398ae1e052c David Lechner 2025-02-07 39 {
> ebb398ae1e052c David Lechner 2025-02-07 40 struct spi_offload_trigger_pwm_state *st = spi_offload_trigger_get_priv(trigger);
> ebb398ae1e052c David Lechner 2025-02-07 41 struct spi_offload_trigger_periodic *periodic = &config->periodic;
> ebb398ae1e052c David Lechner 2025-02-07 42 struct pwm_waveform wf = { };
> ebb398ae1e052c David Lechner 2025-02-07 43 int ret;
> ebb398ae1e052c David Lechner 2025-02-07 44
> ebb398ae1e052c David Lechner 2025-02-07 45 if (config->type != SPI_OFFLOAD_TRIGGER_PERIODIC)
> ebb398ae1e052c David Lechner 2025-02-07 46 return -EINVAL;
> ebb398ae1e052c David Lechner 2025-02-07 47
> ebb398ae1e052c David Lechner 2025-02-07 48 if (!periodic->frequency_hz)
> ebb398ae1e052c David Lechner 2025-02-07 49 return -EINVAL;
> ebb398ae1e052c David Lechner 2025-02-07 50
> ebb398ae1e052c David Lechner 2025-02-07 51 wf.period_length_ns = DIV_ROUND_UP_ULL(NSEC_PER_SEC, periodic->frequency_hz);
> ebb398ae1e052c David Lechner 2025-02-07 52 /* REVISIT: 50% duty-cycle for now - may add config parameter later */
> ebb398ae1e052c David Lechner 2025-02-07 53 wf.duty_length_ns = wf.period_length_ns / 2;
> ebb398ae1e052c David Lechner 2025-02-07 54
> ebb398ae1e052c David Lechner 2025-02-07 @55 ret = pwm_round_waveform_might_sleep(st->pwm, &wf);
> ebb398ae1e052c David Lechner 2025-02-07 56 if (ret < 0)
> ebb398ae1e052c David Lechner 2025-02-07 57 return ret;
> ebb398ae1e052c David Lechner 2025-02-07 58
> ebb398ae1e052c David Lechner 2025-02-07 59 periodic->frequency_hz = DIV_ROUND_UP_ULL(NSEC_PER_SEC, wf.period_length_ns);
> ebb398ae1e052c David Lechner 2025-02-07 60
> ebb398ae1e052c David Lechner 2025-02-07 61 return 0;
> ebb398ae1e052c David Lechner 2025-02-07 62 }
> ebb398ae1e052c David Lechner 2025-02-07 63
> ebb398ae1e052c David Lechner 2025-02-07 64 static int spi_offload_trigger_pwm_enable(struct spi_offload_trigger *trigger,
> ebb398ae1e052c David Lechner 2025-02-07 65 struct spi_offload_trigger_config *config)
> ebb398ae1e052c David Lechner 2025-02-07 66 {
> ebb398ae1e052c David Lechner 2025-02-07 67 struct spi_offload_trigger_pwm_state *st = spi_offload_trigger_get_priv(trigger);
> ebb398ae1e052c David Lechner 2025-02-07 68 struct spi_offload_trigger_periodic *periodic = &config->periodic;
> ebb398ae1e052c David Lechner 2025-02-07 69 struct pwm_waveform wf = { };
> ebb398ae1e052c David Lechner 2025-02-07 70
> ebb398ae1e052c David Lechner 2025-02-07 71 if (config->type != SPI_OFFLOAD_TRIGGER_PERIODIC)
> ebb398ae1e052c David Lechner 2025-02-07 72 return -EINVAL;
> ebb398ae1e052c David Lechner 2025-02-07 73
> ebb398ae1e052c David Lechner 2025-02-07 74 if (!periodic->frequency_hz)
> ebb398ae1e052c David Lechner 2025-02-07 75 return -EINVAL;
> ebb398ae1e052c David Lechner 2025-02-07 76
> ebb398ae1e052c David Lechner 2025-02-07 77 wf.period_length_ns = DIV_ROUND_UP_ULL(NSEC_PER_SEC, periodic->frequency_hz);
> ebb398ae1e052c David Lechner 2025-02-07 78 /* REVISIT: 50% duty-cycle for now - may add config parameter later */
> ebb398ae1e052c David Lechner 2025-02-07 79 wf.duty_length_ns = wf.period_length_ns / 2;
> ebb398ae1e052c David Lechner 2025-02-07 80
> ebb398ae1e052c David Lechner 2025-02-07 @81 return pwm_set_waveform_might_sleep(st->pwm, &wf, false);
> ebb398ae1e052c David Lechner 2025-02-07 82 }
> ebb398ae1e052c David Lechner 2025-02-07 83
> ebb398ae1e052c David Lechner 2025-02-07 84 static void spi_offload_trigger_pwm_disable(struct spi_offload_trigger *trigger)
> ebb398ae1e052c David Lechner 2025-02-07 85 {
> ebb398ae1e052c David Lechner 2025-02-07 86 struct spi_offload_trigger_pwm_state *st = spi_offload_trigger_get_priv(trigger);
> ebb398ae1e052c David Lechner 2025-02-07 87 struct pwm_waveform wf;
> ebb398ae1e052c David Lechner 2025-02-07 88 int ret;
> ebb398ae1e052c David Lechner 2025-02-07 89
> ebb398ae1e052c David Lechner 2025-02-07 @90 ret = pwm_get_waveform_might_sleep(st->pwm, &wf);
> ebb398ae1e052c David Lechner 2025-02-07 91 if (ret < 0) {
> ebb398ae1e052c David Lechner 2025-02-07 92 dev_err(st->dev, "failed to get waveform: %d\n", ret);
> ebb398ae1e052c David Lechner 2025-02-07 93 return;
> ebb398ae1e052c David Lechner 2025-02-07 94 }
> ebb398ae1e052c David Lechner 2025-02-07 95
> ebb398ae1e052c David Lechner 2025-02-07 96 wf.duty_length_ns = 0;
> ebb398ae1e052c David Lechner 2025-02-07 97
> ebb398ae1e052c David Lechner 2025-02-07 98 ret = pwm_set_waveform_might_sleep(st->pwm, &wf, false);
> ebb398ae1e052c David Lechner 2025-02-07 99 if (ret < 0)
> ebb398ae1e052c David Lechner 2025-02-07 100 dev_err(st->dev, "failed to disable PWM: %d\n", ret);
> ebb398ae1e052c David Lechner 2025-02-07 101 }
> ebb398ae1e052c David Lechner 2025-02-07 102
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 6/8] iio: adc: ad4030: Add SPI offload support
2025-09-26 20:40 ` [PATCH v3 6/8] iio: adc: ad4030: Add SPI offload support Marcelo Schmitt
2025-09-27 12:59 ` kernel test robot
@ 2025-09-28 10:08 ` Jonathan Cameron
1 sibling, 0 replies; 23+ messages in thread
From: Jonathan Cameron @ 2025-09-28 10:08 UTC (permalink / raw)
To: Marcelo Schmitt
Cc: linux-iio, devicetree, linux-doc, linux-spi, linux-kernel,
michael.hennerich, nuno.sa, eblanc, dlechner, andy, robh, krzk+dt,
conor+dt, corbet, marcelo.schmitt1, Trevor Gamblin, Axel Haslam
On Fri, 26 Sep 2025 17:40:29 -0300
Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:
> AD4030 and similar ADCs can capture data at sample rates up to 2 mega
> samples per second (MSPS). Not all SPI controllers are able to achieve such
> high throughputs and even when the controller is fast enough to run
> transfers at the required speed, it may be costly to the CPU to handle
> transfer data at such high sample rates. Add SPI offload support for AD4030
> and similar ADCs to enable data capture at maximum sample rates.
>
> Co-developed-by: Trevor Gamblin <tgamblin@baylibre.com>
> Signed-off-by: Trevor Gamblin <tgamblin@baylibre.com>
> Co-developed-by: Axel Haslam <ahaslam@baylibre.com>
> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
This isn't my area of speciality so I'll be looking for some review tags from others.
Comments inline are completely trivial things that'd I'd just fix up but
you'll be doing another spin for the bot error anyway so over to you!
Jonathan
> diff --git a/drivers/iio/adc/ad4030.c b/drivers/iio/adc/ad4030.c
> index cdf5933e9725..8fca98738e3e 100644
> --- a/drivers/iio/adc/ad4030.c
> +++ b/drivers/iio/adc/ad4030.c
> +
> +static int ad4030_update_conversion_rate(struct ad4030_state *st,
> + unsigned int freq, unsigned int avg_log2)
> +{
> + struct spi_offload_trigger_config *config = &st->offload_trigger_config;
> + struct pwm_waveform cnv_wf = { };
> + u64 target = AD4030_TCNVH_NS;
> + u64 offload_period_ns;
> + u64 offload_offset_ns;
> + int ret;
> +
> + /*
> + * When averaging/oversampling over N samples, we fire the offload
> + * trigger once at every N pulses of the CNV signal. Conversely, the CNV
> + * signal needs to be N times faster than the offload trigger. Take that
> + * into account to correctly re-evaluate both the PWM waveform connected
> + * to CNV and the SPI offload trigger.
> + */
> + freq <<= avg_log2;
> +
> + cnv_wf.period_length_ns = DIV_ROUND_CLOSEST(NSEC_PER_SEC, freq);
> + /*
> + * The datasheet lists a minimum time of 9.8 ns, but no maximum. If the
> + * rounded PWM's value is less than 10, increase the target value by 10
> + * and attempt to round the waveform again, until the value is at least
> + * 10 ns. Use a separate variable to represent the target in case the
> + * rounding is severe enough to keep putting the first few results under
> + * the minimum 10ns condition checked by the while loop.
> + */
> + do {
> + cnv_wf.duty_length_ns = target;
> + ret = pwm_round_waveform_might_sleep(st->cnv_trigger, &cnv_wf);
> + if (ret)
> + return ret;
> + target += AD4030_TCNVH_NS;
> + } while (cnv_wf.duty_length_ns < AD4030_TCNVH_NS);
> +
> + if (!in_range(cnv_wf.period_length_ns, AD4030_TCYC_NS, INT_MAX))
> + return -EINVAL;
> +
> + offload_period_ns = cnv_wf.period_length_ns;
> + /*
> + * Make the offload trigger period be N times longer than the CNV PWM
> + * period when averaging over N samples.
> + */
> + offload_period_ns <<= avg_log2;
> +
> + config->periodic.frequency_hz = DIV_ROUND_UP_ULL(NSEC_PER_SEC,
Bonus space after =
> + offload_period_ns);
> @@ -869,7 +1035,9 @@ static int ad4030_get_current_scan_type(const struct iio_dev *indio_dev,
> static int ad4030_update_scan_mode(struct iio_dev *indio_dev,
> const unsigned long *scan_mask)
> {
> - return ad4030_set_mode(indio_dev, *scan_mask);
> + struct ad4030_state *st = iio_priv(indio_dev);
> +
> + return ad4030_set_mode(st, *scan_mask, st->avg_log2);
Trivial and entirely up to you but you can do the following without significant lost of
readability.
return ad4030_set_mode(iio_priv(indio_dev), &scan_mask, st->avg_log2);
> }
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 7/8] dt-bindings: iio: adc: adi,ad4030: Add ADAQ4216 and ADAQ4224
2025-09-26 20:37 [PATCH v3 0/8] Add SPI offload support to AD4030 Marcelo Schmitt
` (5 preceding siblings ...)
2025-09-26 20:40 ` [PATCH v3 6/8] iio: adc: ad4030: Add SPI offload support Marcelo Schmitt
@ 2025-09-26 20:40 ` Marcelo Schmitt
2025-09-26 22:10 ` Rob Herring (Arm)
2025-09-28 10:19 ` Jonathan Cameron
2025-09-26 20:41 ` [PATCH v3 8/8] iio: adc: ad4030: Add support for " Marcelo Schmitt
7 siblings, 2 replies; 23+ messages in thread
From: Marcelo Schmitt @ 2025-09-26 20:40 UTC (permalink / raw)
To: linux-iio, devicetree, linux-doc, linux-spi, linux-kernel
Cc: jic23, michael.hennerich, nuno.sa, eblanc, dlechner, andy, robh,
krzk+dt, conor+dt, corbet, marcelo.schmitt1
ADAQ4216 and ADAQ4224 are similar to AD4030 except that ADAQ devices have a
PGA (programmable gain amplifier) that scales the input signal prior to it
reaching the ADC inputs. The PGA is controlled through a couple of pins (A0
and A1) that set one of four possible signal gain configurations.
Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
Change log v2 -> v3
- PGA gain now described in decibels.
The PGA gain is not going to fit well as a channel property because it may
affect more than one channel as in AD7191.
https://www.analog.com/media/en/technical-documentation/data-sheets/AD7191.pdf
I consulted a very trustworthy source [1, 2] and learned that describing signal
gains in decibels is a common practice. I now think it would be ideal to describe
these PGA and PGA-like gains with properties in decibel units and this patch
is an attempt of doing so. The only problem with this approach is that we end up
with negative values when the gain is lower than 1 (the signal is attenuated)
and device tree specification doesn't support signed integer types. As the
docs being proposed fail dt_binding_check, I guess I have to nack the patch myself.
Any chance of dt specification eventually support signed integers?
Any suggestions appreciated.
[1] https://en.wikipedia.org/wiki/Decibel
[2] https://en.wikipedia.org/wiki/Gain_(electronics)
Thanks,
Marcelo
.../bindings/iio/adc/adi,ad4030.yaml | 84 +++++++++++++++++--
1 file changed, 79 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4030.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4030.yaml
index 564b6f67a96e..20462fa6c39d 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad4030.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4030.yaml
@@ -19,6 +19,8 @@ description: |
* https://www.analog.com/media/en/technical-documentation/data-sheets/ad4030-24-4032-24.pdf
* https://www.analog.com/media/en/technical-documentation/data-sheets/ad4630-24_ad4632-24.pdf
* https://www.analog.com/media/en/technical-documentation/data-sheets/ad4630-16-4632-16.pdf
+ * https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4216.pdf
+ * https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4224.pdf
$ref: /schemas/spi/spi-peripheral-props.yaml#
@@ -31,6 +33,8 @@ properties:
- adi,ad4630-24
- adi,ad4632-16
- adi,ad4632-24
+ - adi,adaq4216
+ - adi,adaq4224
reg:
maxItems: 1
@@ -54,6 +58,14 @@ properties:
description:
Internal buffered Reference. Used when ref-supply is not connected.
+ vddh-supply:
+ description:
+ PGIA Positive Power Supply.
+
+ vdd-fda-supply:
+ description:
+ FDA Positive Power Supply.
+
cnv-gpios:
description:
The Convert Input (CNV). It initiates the sampling conversions.
@@ -64,6 +76,26 @@ properties:
The Reset Input (/RST). Used for asynchronous device reset.
maxItems: 1
+ pga-gpios:
+ description:
+ A0 and A1 pins for gain selection. For devices that have PGA configuration
+ input pins, pga-gpios should be defined if adi,gain-milli is absent.
+ minItems: 2
+ maxItems: 2
+
+ adi,pga-gain-db:
+ description: |
+ Should be present if PGA control inputs are pin-strapped. The values
+ specify the rounded decibel gain calculated from the voltage gain.
+ Possible values:
+ -10 (A1=0, A0=0), (1/3 V/V gain)
+ -5 (A1=0, A0=1), (5/9 V/V gain)
+ 7 (A1=1, A0=0), (20/9 V/V gain)
+ 16 (A1=1, A0=1), (20/3 V/V gain)
+ If defined, pga-gpios must be absent.
+ enum: [-10, -5, 7, 16]
+ default: -10
+
pwms:
description: PWM signal connected to the CNV pin.
maxItems: 1
@@ -86,11 +118,33 @@ required:
- vio-supply
- cnv-gpios
-oneOf:
- - required:
- - ref-supply
- - required:
- - refin-supply
+allOf:
+ - oneOf:
+ - required:
+ - ref-supply
+ - required:
+ - refin-supply
+ # ADAQ devices require a gain property to indicate how hardware PGA is set
+ - if:
+ properties:
+ compatible:
+ contains:
+ pattern: ^adi,adaq
+ then:
+ allOf:
+ - required: [vddh-supply, vdd-fda-supply]
+ properties:
+ ref-supply: false
+ - oneOf:
+ - required:
+ - adi,pga-value
+ - required:
+ - pga-gpios
+ else:
+ properties:
+ adi,pga-value: false
+ pga-gpios: false
+
unevaluatedProperties: false
@@ -114,3 +168,23 @@ examples:
reset-gpios = <&gpio0 1 GPIO_ACTIVE_LOW>;
};
};
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ adc@0 {
+ compatible = "adi,adaq4216";
+ reg = <0>;
+ spi-max-frequency = <80000000>;
+ vdd-5v-supply = <&supply_5V>;
+ vdd-1v8-supply = <&supply_1_8V>;
+ vio-supply = <&supply_1_8V>;
+ ref-supply = <&supply_5V>;
+ cnv-gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>;
+ reset-gpios = <&gpio0 1 GPIO_ACTIVE_LOW>;
+ adi,pga-gain-db = <-5>;
+ };
+ };
+...
--
2.39.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v3 7/8] dt-bindings: iio: adc: adi,ad4030: Add ADAQ4216 and ADAQ4224
2025-09-26 20:40 ` [PATCH v3 7/8] dt-bindings: iio: adc: adi,ad4030: Add ADAQ4216 and ADAQ4224 Marcelo Schmitt
@ 2025-09-26 22:10 ` Rob Herring (Arm)
2025-09-28 10:19 ` Jonathan Cameron
1 sibling, 0 replies; 23+ messages in thread
From: Rob Herring (Arm) @ 2025-09-26 22:10 UTC (permalink / raw)
To: Marcelo Schmitt
Cc: krzk+dt, jic23, conor+dt, corbet, eblanc, devicetree, nuno.sa,
andy, linux-kernel, michael.hennerich, linux-iio,
marcelo.schmitt1, linux-doc, linux-spi, dlechner
On Fri, 26 Sep 2025 17:40:47 -0300, Marcelo Schmitt wrote:
> ADAQ4216 and ADAQ4224 are similar to AD4030 except that ADAQ devices have a
> PGA (programmable gain amplifier) that scales the input signal prior to it
> reaching the ADC inputs. The PGA is controlled through a couple of pins (A0
> and A1) that set one of four possible signal gain configurations.
>
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> ---
> Change log v2 -> v3
> - PGA gain now described in decibels.
>
> The PGA gain is not going to fit well as a channel property because it may
> affect more than one channel as in AD7191.
> https://www.analog.com/media/en/technical-documentation/data-sheets/AD7191.pdf
>
> I consulted a very trustworthy source [1, 2] and learned that describing signal
> gains in decibels is a common practice. I now think it would be ideal to describe
> these PGA and PGA-like gains with properties in decibel units and this patch
> is an attempt of doing so. The only problem with this approach is that we end up
> with negative values when the gain is lower than 1 (the signal is attenuated)
> and device tree specification doesn't support signed integer types. As the
> docs being proposed fail dt_binding_check, I guess I have to nack the patch myself.
> Any chance of dt specification eventually support signed integers?
> Any suggestions appreciated.
>
> [1] https://en.wikipedia.org/wiki/Decibel
> [2] https://en.wikipedia.org/wiki/Gain_(electronics)
>
> Thanks,
> Marcelo
>
> .../bindings/iio/adc/adi,ad4030.yaml | 84 +++++++++++++++++--
> 1 file changed, 79 insertions(+), 5 deletions(-)
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/iio/adc/adi,ad4030.example.dts:68.36-37 syntax error
FATAL ERROR: Unable to parse input tree
make[2]: *** [scripts/Makefile.dtbs:131: Documentation/devicetree/bindings/iio/adc/adi,ad4030.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1527: dt_binding_check] Error 2
make: *** [Makefile:248: __sub-make] Error 2
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/5dc08b622dac1db561f26034c93910ccff75e965.1758916484.git.marcelo.schmitt@analog.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 7/8] dt-bindings: iio: adc: adi,ad4030: Add ADAQ4216 and ADAQ4224
2025-09-26 20:40 ` [PATCH v3 7/8] dt-bindings: iio: adc: adi,ad4030: Add ADAQ4216 and ADAQ4224 Marcelo Schmitt
2025-09-26 22:10 ` Rob Herring (Arm)
@ 2025-09-28 10:19 ` Jonathan Cameron
2025-09-29 14:31 ` Rob Herring
1 sibling, 1 reply; 23+ messages in thread
From: Jonathan Cameron @ 2025-09-28 10:19 UTC (permalink / raw)
To: Marcelo Schmitt
Cc: linux-iio, devicetree, linux-doc, linux-spi, linux-kernel,
michael.hennerich, nuno.sa, eblanc, dlechner, andy, robh, krzk+dt,
conor+dt, corbet, marcelo.schmitt1, Linus Walleij,
Bartosz Golaszewski, linux-gpio
On Fri, 26 Sep 2025 17:40:47 -0300
Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:
> ADAQ4216 and ADAQ4224 are similar to AD4030 except that ADAQ devices have a
> PGA (programmable gain amplifier) that scales the input signal prior to it
> reaching the ADC inputs. The PGA is controlled through a couple of pins (A0
> and A1) that set one of four possible signal gain configurations.
>
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> ---
> Change log v2 -> v3
> - PGA gain now described in decibels.
>
> The PGA gain is not going to fit well as a channel property because it may
> affect more than one channel as in AD7191.
> https://www.analog.com/media/en/technical-documentation/data-sheets/AD7191.pdf
>
> I consulted a very trustworthy source [1, 2] and learned that describing signal
> gains in decibels is a common practice. I now think it would be ideal to describe
> these PGA and PGA-like gains with properties in decibel units and this patch
> is an attempt of doing so. The only problem with this approach is that we end up
> with negative values when the gain is lower than 1 (the signal is attenuated)
> and device tree specification doesn't support signed integer types. As the
> docs being proposed fail dt_binding_check, I guess I have to nack the patch myself.
> Any chance of dt specification eventually support signed integers?
> Any suggestions appreciated.
>
> [1] https://en.wikipedia.org/wiki/Decibel
> [2] https://en.wikipedia.org/wiki/Gain_(electronics)
I still wonder if the better way to describe this is to ignore that it
has anything to do with PGA as such and instead describe the pin strapping.
DT folk, is there an existing way to do that? My grep skills are failing to
spot one.
We've papered over this for a long time in various IIO drivers by controlling
directly what the pin strap controls with weird and wonderful device specific
bindings. I wonder if we can't have a gpio driver + binding that rejects all
config and just lets us check the current state of an output pin. Kind of a
fixed mode regulator equivalent for gpios.
+CC Linus, Bartosz and gpio list.
>
> Thanks,
> Marcelo
>
> .../bindings/iio/adc/adi,ad4030.yaml | 84 +++++++++++++++++--
> 1 file changed, 79 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4030.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4030.yaml
> index 564b6f67a96e..20462fa6c39d 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad4030.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4030.yaml
> @@ -19,6 +19,8 @@ description: |
> * https://www.analog.com/media/en/technical-documentation/data-sheets/ad4030-24-4032-24.pdf
> * https://www.analog.com/media/en/technical-documentation/data-sheets/ad4630-24_ad4632-24.pdf
> * https://www.analog.com/media/en/technical-documentation/data-sheets/ad4630-16-4632-16.pdf
> + * https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4216.pdf
> + * https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4224.pdf
>
> $ref: /schemas/spi/spi-peripheral-props.yaml#
>
> @@ -31,6 +33,8 @@ properties:
> - adi,ad4630-24
> - adi,ad4632-16
> - adi,ad4632-24
> + - adi,adaq4216
> + - adi,adaq4224
>
> reg:
> maxItems: 1
> @@ -54,6 +58,14 @@ properties:
> description:
> Internal buffered Reference. Used when ref-supply is not connected.
>
> + vddh-supply:
> + description:
> + PGIA Positive Power Supply.
> +
> + vdd-fda-supply:
> + description:
> + FDA Positive Power Supply.
> +
> cnv-gpios:
> description:
> The Convert Input (CNV). It initiates the sampling conversions.
> @@ -64,6 +76,26 @@ properties:
> The Reset Input (/RST). Used for asynchronous device reset.
> maxItems: 1
>
> + pga-gpios:
> + description:
> + A0 and A1 pins for gain selection. For devices that have PGA configuration
> + input pins, pga-gpios should be defined if adi,gain-milli is absent.
> + minItems: 2
> + maxItems: 2
> +
> + adi,pga-gain-db:
> + description: |
> + Should be present if PGA control inputs are pin-strapped. The values
> + specify the rounded decibel gain calculated from the voltage gain.
> + Possible values:
> + -10 (A1=0, A0=0), (1/3 V/V gain)
> + -5 (A1=0, A0=1), (5/9 V/V gain)
> + 7 (A1=1, A0=0), (20/9 V/V gain)
> + 16 (A1=1, A0=1), (20/3 V/V gain)
> + If defined, pga-gpios must be absent.
> + enum: [-10, -5, 7, 16]
> + default: -10
> +
> pwms:
> description: PWM signal connected to the CNV pin.
> maxItems: 1
> @@ -86,11 +118,33 @@ required:
> - vio-supply
> - cnv-gpios
>
> -oneOf:
> - - required:
> - - ref-supply
> - - required:
> - - refin-supply
> +allOf:
> + - oneOf:
> + - required:
> + - ref-supply
> + - required:
> + - refin-supply
> + # ADAQ devices require a gain property to indicate how hardware PGA is set
> + - if:
> + properties:
> + compatible:
> + contains:
> + pattern: ^adi,adaq
> + then:
> + allOf:
> + - required: [vddh-supply, vdd-fda-supply]
> + properties:
> + ref-supply: false
> + - oneOf:
> + - required:
> + - adi,pga-value
> + - required:
> + - pga-gpios
> + else:
> + properties:
> + adi,pga-value: false
> + pga-gpios: false
> +
>
> unevaluatedProperties: false
>
> @@ -114,3 +168,23 @@ examples:
> reset-gpios = <&gpio0 1 GPIO_ACTIVE_LOW>;
> };
> };
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + adc@0 {
> + compatible = "adi,adaq4216";
> + reg = <0>;
> + spi-max-frequency = <80000000>;
> + vdd-5v-supply = <&supply_5V>;
> + vdd-1v8-supply = <&supply_1_8V>;
> + vio-supply = <&supply_1_8V>;
> + ref-supply = <&supply_5V>;
> + cnv-gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>;
> + reset-gpios = <&gpio0 1 GPIO_ACTIVE_LOW>;
> + adi,pga-gain-db = <-5>;
> + };
> + };
> +...
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v3 7/8] dt-bindings: iio: adc: adi,ad4030: Add ADAQ4216 and ADAQ4224
2025-09-28 10:19 ` Jonathan Cameron
@ 2025-09-29 14:31 ` Rob Herring
2025-09-29 16:16 ` David Lechner
0 siblings, 1 reply; 23+ messages in thread
From: Rob Herring @ 2025-09-29 14:31 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Marcelo Schmitt, linux-iio, devicetree, linux-doc, linux-spi,
linux-kernel, michael.hennerich, nuno.sa, eblanc, dlechner, andy,
krzk+dt, conor+dt, corbet, marcelo.schmitt1, Linus Walleij,
Bartosz Golaszewski, linux-gpio
On Sun, Sep 28, 2025 at 11:19:55AM +0100, Jonathan Cameron wrote:
> On Fri, 26 Sep 2025 17:40:47 -0300
> Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:
>
> > ADAQ4216 and ADAQ4224 are similar to AD4030 except that ADAQ devices have a
> > PGA (programmable gain amplifier) that scales the input signal prior to it
> > reaching the ADC inputs. The PGA is controlled through a couple of pins (A0
> > and A1) that set one of four possible signal gain configurations.
> >
> > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > ---
> > Change log v2 -> v3
> > - PGA gain now described in decibels.
> >
> > The PGA gain is not going to fit well as a channel property because it may
> > affect more than one channel as in AD7191.
> > https://www.analog.com/media/en/technical-documentation/data-sheets/AD7191.pdf
> >
> > I consulted a very trustworthy source [1, 2] and learned that describing signal
> > gains in decibels is a common practice. I now think it would be ideal to describe
> > these PGA and PGA-like gains with properties in decibel units and this patch
> > is an attempt of doing so. The only problem with this approach is that we end up
> > with negative values when the gain is lower than 1 (the signal is attenuated)
> > and device tree specification doesn't support signed integer types. As the
> > docs being proposed fail dt_binding_check, I guess I have to nack the patch myself.
> > Any chance of dt specification eventually support signed integers?
> > Any suggestions appreciated.
> >
> > [1] https://en.wikipedia.org/wiki/Decibel
> > [2] https://en.wikipedia.org/wiki/Gain_(electronics)
>
> I still wonder if the better way to describe this is to ignore that it
> has anything to do with PGA as such and instead describe the pin strapping.
>
> DT folk, is there an existing way to do that? My grep skills are failing to
> spot one.
>
> We've papered over this for a long time in various IIO drivers by controlling
> directly what the pin strap controls with weird and wonderful device specific
> bindings. I wonder if we can't have a gpio driver + binding that rejects all
> config and just lets us check the current state of an output pin. Kind of a
> fixed mode regulator equivalent for gpios.
If these are connected to GPIOs, isn't it possible that someone will
want to change their value?
Other than some generic 'pinstrap-gpios' property, I don't see what we'd
do here? I don't feel like pin strapping GPIOs is something that we see
all that often.
Rob
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 7/8] dt-bindings: iio: adc: adi,ad4030: Add ADAQ4216 and ADAQ4224
2025-09-29 14:31 ` Rob Herring
@ 2025-09-29 16:16 ` David Lechner
2025-09-30 14:47 ` Marcelo Schmitt
2025-09-30 18:26 ` Rob Herring
0 siblings, 2 replies; 23+ messages in thread
From: David Lechner @ 2025-09-29 16:16 UTC (permalink / raw)
To: Rob Herring
Cc: Jonathan Cameron, Marcelo Schmitt, linux-iio, devicetree,
linux-doc, linux-spi, linux-kernel, michael.hennerich, nuno.sa,
eblanc, andy, krzk+dt, conor+dt, corbet, marcelo.schmitt1,
Linus Walleij, Bartosz Golaszewski, linux-gpio
On Mon, Sep 29, 2025 at 4:31 PM Rob Herring <robh@kernel.org> wrote:
>
> On Sun, Sep 28, 2025 at 11:19:55AM +0100, Jonathan Cameron wrote:
> > On Fri, 26 Sep 2025 17:40:47 -0300
> > Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:
> >
> > > ADAQ4216 and ADAQ4224 are similar to AD4030 except that ADAQ devices have a
> > > PGA (programmable gain amplifier) that scales the input signal prior to it
> > > reaching the ADC inputs. The PGA is controlled through a couple of pins (A0
> > > and A1) that set one of four possible signal gain configurations.
> > >
> > > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > > ---
> > > Change log v2 -> v3
> > > - PGA gain now described in decibels.
> > >
> > > The PGA gain is not going to fit well as a channel property because it may
> > > affect more than one channel as in AD7191.
> > > https://www.analog.com/media/en/technical-documentation/data-sheets/AD7191.pdf
> > >
> > > I consulted a very trustworthy source [1, 2] and learned that describing signal
> > > gains in decibels is a common practice. I now think it would be ideal to describe
> > > these PGA and PGA-like gains with properties in decibel units and this patch
> > > is an attempt of doing so. The only problem with this approach is that we end up
> > > with negative values when the gain is lower than 1 (the signal is attenuated)
> > > and device tree specification doesn't support signed integer types. As the
> > > docs being proposed fail dt_binding_check, I guess I have to nack the patch myself.
> > > Any chance of dt specification eventually support signed integers?
> > > Any suggestions appreciated.
> > >
> > > [1] https://en.wikipedia.org/wiki/Decibel
> > > [2] https://en.wikipedia.org/wiki/Gain_(electronics)
> >
> > I still wonder if the better way to describe this is to ignore that it
> > has anything to do with PGA as such and instead describe the pin strapping.
> >
> > DT folk, is there an existing way to do that? My grep skills are failing to
> > spot one.
> >
> > We've papered over this for a long time in various IIO drivers by controlling
> > directly what the pin strap controls with weird and wonderful device specific
> > bindings. I wonder if we can't have a gpio driver + binding that rejects all
> > config and just lets us check the current state of an output pin. Kind of a
> > fixed mode regulator equivalent for gpios.
>
> If these are connected to GPIOs, isn't it possible that someone will
> want to change their value?
>
> Other than some generic 'pinstrap-gpios' property, I don't see what we'd
> do here? I don't feel like pin strapping GPIOs is something that we see
> all that often.
>
> Rob
I think the idea is that it is not actually a GPIO, just a hard-wired
connection. We would want to have a "fixed-gpios" to describe these
hard-wired connections as GPIOs so that we don't have to write complex
binding for chip config GPIOs. I've seen configuration pins like on at
least half a dozed of the ADCs I've been working on/reviewing over the
last two years (since I got involved in IIO again).
For example, there might be 4 mode pins, so we would like to just have
a mode-gpios property. So this could be all 4 connected to GPIOs, all
4 hard-wired, or a mix.
(The actual bindings would need more thought, but this should give the
general idea)
fixed_gpio: hard-wires {
compatible = "fixed-gpios";
gpio-controller;
#gpio-cells = <1>;
};
gpio0: gpio-controller@4000000 {
compatible = "vendor,soc-gpios";
gpio-controller;
#gpio-cells = <2>;
};
spi {
adc@0 {
compatible = "vendor,adc";
/* All gpios */
mode-gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>,
<&gpio0 1 GPIO_ACTIVE_HIGH>,
<&gpio0 2 GPIO_ACTIVE_HIGH>,
<&gpio0 3 GPIO_ACTIVE_HIGH>;
/* or all hard-wired */
mode-gpios = <&fixed_gpio 0 GPIO_FIXED_HIGH>,
<&fixed_gpio GPIO_FIXED_HIGH>,
<&fixed_gpio GPIO_FIXED_LOW>,
<&fixed_gpio GPIO_FIXED_LOW>;
/* or mixed */
mode-gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>,
<&gpio0 1 GPIO_ACTIVE_HIGH>,
<&fixed_gpio GPIO_FIXED_LOW>,
<&fixed_gpio GPIO_FIXED_LOW>;
};
};
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v3 7/8] dt-bindings: iio: adc: adi,ad4030: Add ADAQ4216 and ADAQ4224
2025-09-29 16:16 ` David Lechner
@ 2025-09-30 14:47 ` Marcelo Schmitt
2025-09-30 17:02 ` Marcelo Schmitt
2025-09-30 18:26 ` Rob Herring
1 sibling, 1 reply; 23+ messages in thread
From: Marcelo Schmitt @ 2025-09-30 14:47 UTC (permalink / raw)
To: David Lechner
Cc: Rob Herring, Jonathan Cameron, Marcelo Schmitt, linux-iio,
devicetree, linux-doc, linux-spi, linux-kernel, michael.hennerich,
nuno.sa, eblanc, andy, krzk+dt, conor+dt, corbet, Linus Walleij,
Bartosz Golaszewski, linux-gpio
On 09/29, David Lechner wrote:
> On Mon, Sep 29, 2025 at 4:31 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Sun, Sep 28, 2025 at 11:19:55AM +0100, Jonathan Cameron wrote:
> > > On Fri, 26 Sep 2025 17:40:47 -0300
> > > Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:
> > >
> > > > ADAQ4216 and ADAQ4224 are similar to AD4030 except that ADAQ devices have a
> > > > PGA (programmable gain amplifier) that scales the input signal prior to it
> > > > reaching the ADC inputs. The PGA is controlled through a couple of pins (A0
> > > > and A1) that set one of four possible signal gain configurations.
> > > >
> > > > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > > > ---
> > > > Change log v2 -> v3
> > > > - PGA gain now described in decibels.
> > > >
> > > > The PGA gain is not going to fit well as a channel property because it may
> > > > affect more than one channel as in AD7191.
> > > > https://www.analog.com/media/en/technical-documentation/data-sheets/AD7191.pdf
> > > >
> > > > I consulted a very trustworthy source [1, 2] and learned that describing signal
> > > > gains in decibels is a common practice. I now think it would be ideal to describe
> > > > these PGA and PGA-like gains with properties in decibel units and this patch
> > > > is an attempt of doing so. The only problem with this approach is that we end up
> > > > with negative values when the gain is lower than 1 (the signal is attenuated)
> > > > and device tree specification doesn't support signed integer types. As the
> > > > docs being proposed fail dt_binding_check, I guess I have to nack the patch myself.
> > > > Any chance of dt specification eventually support signed integers?
> > > > Any suggestions appreciated.
> > > >
> > > > [1] https://en.wikipedia.org/wiki/Decibel
> > > > [2] https://en.wikipedia.org/wiki/Gain_(electronics)
> > >
> > > I still wonder if the better way to describe this is to ignore that it
> > > has anything to do with PGA as such and instead describe the pin strapping.
> > >
> > > DT folk, is there an existing way to do that? My grep skills are failing to
> > > spot one.
> > >
> > > We've papered over this for a long time in various IIO drivers by controlling
> > > directly what the pin strap controls with weird and wonderful device specific
> > > bindings. I wonder if we can't have a gpio driver + binding that rejects all
> > > config and just lets us check the current state of an output pin. Kind of a
> > > fixed mode regulator equivalent for gpios.
> >
> > If these are connected to GPIOs, isn't it possible that someone will
> > want to change their value?
> >
> > Other than some generic 'pinstrap-gpios' property, I don't see what we'd
> > do here? I don't feel like pin strapping GPIOs is something that we see
> > all that often.
> >
> > Rob
>
> I think the idea is that it is not actually a GPIO, just a hard-wired
> connection. We would want to have a "fixed-gpios" to describe these
> hard-wired connections as GPIOs so that we don't have to write complex
> binding for chip config GPIOs. I've seen configuration pins like on at
> least half a dozed of the ADCs I've been working on/reviewing over the
> last two years (since I got involved in IIO again).
Yes, the alternative to having GPIOs would be to have pins hard-wired set to a
specific logic level. And the connection don't need to be to GPIOs. The gain
pins on the ADC chip can be connected to anything that keeps a constant logic
level while we capture data from the ADC.
>
> For example, there might be 4 mode pins, so we would like to just have
> a mode-gpios property. So this could be all 4 connected to GPIOs, all
> 4 hard-wired, or a mix.
Having some pins hard-wired and some connected to GPIOs is possible, but that
would make things even more complex as each pin on the ADC chip sets a different
portion of the gain. IMHO, mixed GPIO/hard-wired configuration starts looking
like over engineering and I haven't been requested for so much configuration
flexibility. Having either all hard-wired or all connected to GPIOs should be ok.
I'm not familiar with pinctrl dt-bindings, but I was wondering if we could get
to something similar with pinctrl. Based on some pinctrl bindings, I think
fixed-level GPIOs could look like the following (for the 4 pin-mode example).
pinctrl0: pincontroller@0 {
compatible = "vendor,model-pinctrl";
all-low-state: some-gpio-pins {
pins = "gpio0", "gpio1", "gpio2", "gpio3";
function = "gpio";
output-low;
};
all-high-state: some-gpio-pins {
pins = "gpio0", "gpio1", "gpio2", "gpio3";
function = "gpio";
output-high;
};
most-high-state: some-gpio-pins {
pins1 {
pins = "gpio0", "gpio1", "gpio2";
function = "gpio";
output-high;
};
pins2 {
pins = "gpio3";
function = "gpio";
output-low;
};
};
};
spi {
adc@0 {
compatible = "vendor,adc";
/* All gpios */
pga-gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>,
<&gpio0 1 GPIO_ACTIVE_HIGH>,
<&gpio0 2 GPIO_ACTIVE_HIGH>,
<&gpio0 3 GPIO_ACTIVE_HIGH>;
/* or all hard-wired */
pinctrl-names = "minimum-gain", "moderate-gain", "maximum-gain";
pinctrl-0 = <&all-low-state>, <&most-high-state>, <&all-high-state>;
};
};
Though, the above is still relying on GPIOs which is not a requirement from
ADC peripheral perspective. Also, if GPIOs are available, one can just provide
them through pga-gpios and have full control over the signal gain with the IIO
driver. It boils down to just telling software what are the logical levels at
two pins on the ADC chip when GPIOs are not provided.
Thanks,
Marcelo
>
> (The actual bindings would need more thought, but this should give the
> general idea)
>
> fixed_gpio: hard-wires {
> compatible = "fixed-gpios";
> gpio-controller;
> #gpio-cells = <1>;
> };
>
> gpio0: gpio-controller@4000000 {
> compatible = "vendor,soc-gpios";
> gpio-controller;
> #gpio-cells = <2>;
> };
>
> spi {
> adc@0 {
> compatible = "vendor,adc";
> /* All gpios */
> mode-gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>,
> <&gpio0 1 GPIO_ACTIVE_HIGH>,
> <&gpio0 2 GPIO_ACTIVE_HIGH>,
> <&gpio0 3 GPIO_ACTIVE_HIGH>;
> /* or all hard-wired */
> mode-gpios = <&fixed_gpio 0 GPIO_FIXED_HIGH>,
> <&fixed_gpio GPIO_FIXED_HIGH>,
> <&fixed_gpio GPIO_FIXED_LOW>,
> <&fixed_gpio GPIO_FIXED_LOW>;
> /* or mixed */
> mode-gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>,
> <&gpio0 1 GPIO_ACTIVE_HIGH>,
> <&fixed_gpio GPIO_FIXED_LOW>,
> <&fixed_gpio GPIO_FIXED_LOW>;
> };
> };
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v3 7/8] dt-bindings: iio: adc: adi,ad4030: Add ADAQ4216 and ADAQ4224
2025-09-30 14:47 ` Marcelo Schmitt
@ 2025-09-30 17:02 ` Marcelo Schmitt
0 siblings, 0 replies; 23+ messages in thread
From: Marcelo Schmitt @ 2025-09-30 17:02 UTC (permalink / raw)
To: David Lechner
Cc: Rob Herring, Jonathan Cameron, Marcelo Schmitt, linux-iio,
devicetree, linux-doc, linux-spi, linux-kernel, michael.hennerich,
nuno.sa, eblanc, andy, krzk+dt, conor+dt, corbet, Linus Walleij,
Bartosz Golaszewski, linux-gpio
...
> > > > > ADAQ4216 and ADAQ4224 are similar to AD4030 except that ADAQ devices have a
> > > > > PGA (programmable gain amplifier) that scales the input signal prior to it
> > > > > reaching the ADC inputs. The PGA is controlled through a couple of pins (A0
> > > > > and A1) that set one of four possible signal gain configurations.
> > > > >
> > > > > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > > > > ---
> > > > > Change log v2 -> v3
> > > > > - PGA gain now described in decibels.
> > > > >
> > > > > The PGA gain is not going to fit well as a channel property because it may
> > > > > affect more than one channel as in AD7191.
> > > > > https://www.analog.com/media/en/technical-documentation/data-sheets/AD7191.pdf
> > > > >
> > > > > I consulted a very trustworthy source [1, 2] and learned that describing signal
> > > > > gains in decibels is a common practice. I now think it would be ideal to describe
> > > > > these PGA and PGA-like gains with properties in decibel units and this patch
> > > > > is an attempt of doing so. The only problem with this approach is that we end up
> > > > > with negative values when the gain is lower than 1 (the signal is attenuated)
> > > > > and device tree specification doesn't support signed integer types. As the
> > > > > docs being proposed fail dt_binding_check, I guess I have to nack the patch myself.
> > > > > Any chance of dt specification eventually support signed integers?
> > > > > Any suggestions appreciated.
> > > > >
> > > > > [1] https://en.wikipedia.org/wiki/Decibel
> > > > > [2] https://en.wikipedia.org/wiki/Gain_(electronics)
> > > >
...
>
> Though, the above is still relying on GPIOs which is not a requirement from
> ADC peripheral perspective. Also, if GPIOs are available, one can just provide
> them through pga-gpios and have full control over the signal gain with the IIO
> driver. It boils down to just telling software what are the logical levels at
> two pins on the ADC chip when GPIOs are not provided.
>
Though, as mentioned, the state of A0 and A1 pins defines a certain gain applied
to ADC input signal. Because signal gains seem to be usually described in decibels,
the proposed dt-binding allows to provide the gain value in decibels and then
software figures out what A0 and A1 logical levels are from the provided decibels.
The actual levels of A0 and A1 then have to be set according to the provided
decibel gain.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 7/8] dt-bindings: iio: adc: adi,ad4030: Add ADAQ4216 and ADAQ4224
2025-09-29 16:16 ` David Lechner
2025-09-30 14:47 ` Marcelo Schmitt
@ 2025-09-30 18:26 ` Rob Herring
2025-10-01 11:55 ` David Lechner
1 sibling, 1 reply; 23+ messages in thread
From: Rob Herring @ 2025-09-30 18:26 UTC (permalink / raw)
To: David Lechner
Cc: Jonathan Cameron, Marcelo Schmitt, linux-iio, devicetree,
linux-doc, linux-spi, linux-kernel, michael.hennerich, nuno.sa,
eblanc, andy, krzk+dt, conor+dt, corbet, marcelo.schmitt1,
Linus Walleij, Bartosz Golaszewski, linux-gpio
On Mon, Sep 29, 2025 at 06:16:10PM +0200, David Lechner wrote:
> On Mon, Sep 29, 2025 at 4:31 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Sun, Sep 28, 2025 at 11:19:55AM +0100, Jonathan Cameron wrote:
> > > On Fri, 26 Sep 2025 17:40:47 -0300
> > > Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:
> > >
> > > > ADAQ4216 and ADAQ4224 are similar to AD4030 except that ADAQ devices have a
> > > > PGA (programmable gain amplifier) that scales the input signal prior to it
> > > > reaching the ADC inputs. The PGA is controlled through a couple of pins (A0
> > > > and A1) that set one of four possible signal gain configurations.
> > > >
> > > > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > > > ---
> > > > Change log v2 -> v3
> > > > - PGA gain now described in decibels.
> > > >
> > > > The PGA gain is not going to fit well as a channel property because it may
> > > > affect more than one channel as in AD7191.
> > > > https://www.analog.com/media/en/technical-documentation/data-sheets/AD7191.pdf
> > > >
> > > > I consulted a very trustworthy source [1, 2] and learned that describing signal
> > > > gains in decibels is a common practice. I now think it would be ideal to describe
> > > > these PGA and PGA-like gains with properties in decibel units and this patch
> > > > is an attempt of doing so. The only problem with this approach is that we end up
> > > > with negative values when the gain is lower than 1 (the signal is attenuated)
> > > > and device tree specification doesn't support signed integer types. As the
> > > > docs being proposed fail dt_binding_check, I guess I have to nack the patch myself.
> > > > Any chance of dt specification eventually support signed integers?
> > > > Any suggestions appreciated.
> > > >
> > > > [1] https://en.wikipedia.org/wiki/Decibel
> > > > [2] https://en.wikipedia.org/wiki/Gain_(electronics)
> > >
> > > I still wonder if the better way to describe this is to ignore that it
> > > has anything to do with PGA as such and instead describe the pin strapping.
> > >
> > > DT folk, is there an existing way to do that? My grep skills are failing to
> > > spot one.
> > >
> > > We've papered over this for a long time in various IIO drivers by controlling
> > > directly what the pin strap controls with weird and wonderful device specific
> > > bindings. I wonder if we can't have a gpio driver + binding that rejects all
> > > config and just lets us check the current state of an output pin. Kind of a
> > > fixed mode regulator equivalent for gpios.
> >
> > If these are connected to GPIOs, isn't it possible that someone will
> > want to change their value?
> >
> > Other than some generic 'pinstrap-gpios' property, I don't see what we'd
> > do here? I don't feel like pin strapping GPIOs is something that we see
> > all that often.
> >
> > Rob
>
> I think the idea is that it is not actually a GPIO, just a hard-wired
> connection. We would want to have a "fixed-gpios" to describe these
> hard-wired connections as GPIOs so that we don't have to write complex
> binding for chip config GPIOs. I've seen configuration pins like on at
> least half a dozed of the ADCs I've been working on/reviewing over the
> last two years (since I got involved in IIO again).
Until I read the example, I totally missed what you want here...
Can you point me to some existing bindings?
IIRC, Linus has expressed not caring for cases of using GPIO API on
things that are not GPIOs. That was more like registers which can
read the state of signals. Better let him weigh in before we go too far
down this path.
>
> For example, there might be 4 mode pins, so we would like to just have
> a mode-gpios property. So this could be all 4 connected to GPIOs, all
> 4 hard-wired, or a mix.
>
> (The actual bindings would need more thought, but this should give the
> general idea)
>
> fixed_gpio: hard-wires {
> compatible = "fixed-gpios";
> gpio-controller;
> #gpio-cells = <1>;
> };
>
> gpio0: gpio-controller@4000000 {
> compatible = "vendor,soc-gpios";
> gpio-controller;
> #gpio-cells = <2>;
> };
>
> spi {
> adc@0 {
> compatible = "vendor,adc";
> /* All gpios */
> mode-gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>,
> <&gpio0 1 GPIO_ACTIVE_HIGH>,
> <&gpio0 2 GPIO_ACTIVE_HIGH>,
> <&gpio0 3 GPIO_ACTIVE_HIGH>;
> /* or all hard-wired */
> mode-gpios = <&fixed_gpio 0 GPIO_FIXED_HIGH>,
> <&fixed_gpio GPIO_FIXED_HIGH>,
> <&fixed_gpio GPIO_FIXED_LOW>,
> <&fixed_gpio GPIO_FIXED_LOW>;
> /* or mixed */
> mode-gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>,
> <&gpio0 1 GPIO_ACTIVE_HIGH>,
> <&fixed_gpio GPIO_FIXED_LOW>,
> <&fixed_gpio GPIO_FIXED_LOW>;
The above seems reasonable to me.
Just to throw out an alternative, phandle values of 0 and -1 are
generally reserved. Historically that means just skip the entry.
However, you could use that and do something like this:
mode-gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>,
<&gpio0 1 GPIO_ACTIVE_HIGH>,
<0>,
<0xffffffff>;
So 0 means low and ~0 means high. The only advantage I see with it is
you don't need a "fixed-gpios" driver. Also, I'm not sure how that would
work with requesting GPIOs given you've essentially defined only 2 GPIO
lines (high and low). Though Bartosz is doing some work on non-exclusive
GPIOs.
Rob
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v3 7/8] dt-bindings: iio: adc: adi,ad4030: Add ADAQ4216 and ADAQ4224
2025-09-30 18:26 ` Rob Herring
@ 2025-10-01 11:55 ` David Lechner
0 siblings, 0 replies; 23+ messages in thread
From: David Lechner @ 2025-10-01 11:55 UTC (permalink / raw)
To: Rob Herring
Cc: Jonathan Cameron, Marcelo Schmitt, linux-iio, devicetree,
linux-doc, linux-spi, linux-kernel, michael.hennerich, nuno.sa,
eblanc, andy, krzk+dt, conor+dt, corbet, marcelo.schmitt1,
Linus Walleij, Bartosz Golaszewski, linux-gpio
On Tue, Sep 30, 2025 at 8:26 PM Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Sep 29, 2025 at 06:16:10PM +0200, David Lechner wrote:
> > On Mon, Sep 29, 2025 at 4:31 PM Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Sun, Sep 28, 2025 at 11:19:55AM +0100, Jonathan Cameron wrote:
> > > > On Fri, 26 Sep 2025 17:40:47 -0300
> > > > Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:
> > > >
> > > > > ADAQ4216 and ADAQ4224 are similar to AD4030 except that ADAQ devices have a
> > > > > PGA (programmable gain amplifier) that scales the input signal prior to it
> > > > > reaching the ADC inputs. The PGA is controlled through a couple of pins (A0
> > > > > and A1) that set one of four possible signal gain configurations.
> > > > >
> > > > > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > > > > ---
> > > > > Change log v2 -> v3
> > > > > - PGA gain now described in decibels.
> > > > >
> > > > > The PGA gain is not going to fit well as a channel property because it may
> > > > > affect more than one channel as in AD7191.
> > > > > https://www.analog.com/media/en/technical-documentation/data-sheets/AD7191.pdf
> > > > >
> > > > > I consulted a very trustworthy source [1, 2] and learned that describing signal
> > > > > gains in decibels is a common practice. I now think it would be ideal to describe
> > > > > these PGA and PGA-like gains with properties in decibel units and this patch
> > > > > is an attempt of doing so. The only problem with this approach is that we end up
> > > > > with negative values when the gain is lower than 1 (the signal is attenuated)
> > > > > and device tree specification doesn't support signed integer types. As the
> > > > > docs being proposed fail dt_binding_check, I guess I have to nack the patch myself.
> > > > > Any chance of dt specification eventually support signed integers?
> > > > > Any suggestions appreciated.
> > > > >
> > > > > [1] https://en.wikipedia.org/wiki/Decibel
> > > > > [2] https://en.wikipedia.org/wiki/Gain_(electronics)
> > > >
> > > > I still wonder if the better way to describe this is to ignore that it
> > > > has anything to do with PGA as such and instead describe the pin strapping.
> > > >
> > > > DT folk, is there an existing way to do that? My grep skills are failing to
> > > > spot one.
> > > >
> > > > We've papered over this for a long time in various IIO drivers by controlling
> > > > directly what the pin strap controls with weird and wonderful device specific
> > > > bindings. I wonder if we can't have a gpio driver + binding that rejects all
> > > > config and just lets us check the current state of an output pin. Kind of a
> > > > fixed mode regulator equivalent for gpios.
> > >
> > > If these are connected to GPIOs, isn't it possible that someone will
> > > want to change their value?
> > >
> > > Other than some generic 'pinstrap-gpios' property, I don't see what we'd
> > > do here? I don't feel like pin strapping GPIOs is something that we see
> > > all that often.
> > >
> > > Rob
> >
> > I think the idea is that it is not actually a GPIO, just a hard-wired
> > connection. We would want to have a "fixed-gpios" to describe these
> > hard-wired connections as GPIOs so that we don't have to write complex
> > binding for chip config GPIOs. I've seen configuration pins like on at
> > least half a dozed of the ADCs I've been working on/reviewing over the
> > last two years (since I got involved in IIO again).
>
> Until I read the example, I totally missed what you want here...
>
> Can you point me to some existing bindings?
Perhaps the best example is adi,ad7194.yaml [1]. It has odr-gpios for
a 2 pin input to select 4 possible ODR values in the case where they
are connected to gpios and can be configured at runtime. Then it has a
separate adi,odr-value property to give the hardwired value in cases
where they are not connected to gpios. The binding currently doesn't
allow having one pin connected to a gpio and one hardwired. The same
binding also has pga-gpios and adi,pga-value which work the same and
just control a different configuration parameter.
adi,ad7606.yaml [2] is a bit less complete. It has
adi,oversampling-ratio-gpios but it only has adi,sw-mode to indicate
that all 3 oversampling pins are hard-wired high. It doesn't have a
way to specify other hard-wired states. IIRC, the AD7616 chip in this
family also has some more similar config selection pins that aren't
documented yet.
In adi,ad7625 [3], we ended up making 4 enX-gpios for single
properties plus a adi,en0-always-on boolean flag property for each ENX
pin instead of a en-gpios array of 4 gpios. This was a case where it
was highly likely that there would be a mix of hard-wired pins and
gpio-connected pins, so it seemed to be the simplest way to describe
it at the time. It would have been much more ergonomic though if we
could have used the single array.
[1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml#n52
[2]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml#n127
[3]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/iio/adc/adi,ad7625.yaml#n70
>
> IIRC, Linus has expressed not caring for cases of using GPIO API on
> things that are not GPIOs. That was more like registers which can
> read the state of signals. Better let him weigh in before we go too far
> down this path.
>
> >
> > For example, there might be 4 mode pins, so we would like to just have
> > a mode-gpios property. So this could be all 4 connected to GPIOs, all
> > 4 hard-wired, or a mix.
> >
> > (The actual bindings would need more thought, but this should give the
> > general idea)
> >
> > fixed_gpio: hard-wires {
> > compatible = "fixed-gpios";
> > gpio-controller;
> > #gpio-cells = <1>;
> > };
> >
> > gpio0: gpio-controller@4000000 {
> > compatible = "vendor,soc-gpios";
> > gpio-controller;
> > #gpio-cells = <2>;
> > };
> >
> > spi {
> > adc@0 {
> > compatible = "vendor,adc";
> > /* All gpios */
> > mode-gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>,
> > <&gpio0 1 GPIO_ACTIVE_HIGH>,
> > <&gpio0 2 GPIO_ACTIVE_HIGH>,
> > <&gpio0 3 GPIO_ACTIVE_HIGH>;
> > /* or all hard-wired */
> > mode-gpios = <&fixed_gpio 0 GPIO_FIXED_HIGH>,
> > <&fixed_gpio GPIO_FIXED_HIGH>,
> > <&fixed_gpio GPIO_FIXED_LOW>,
> > <&fixed_gpio GPIO_FIXED_LOW>;
> > /* or mixed */
> > mode-gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>,
> > <&gpio0 1 GPIO_ACTIVE_HIGH>,
> > <&fixed_gpio GPIO_FIXED_LOW>,
> > <&fixed_gpio GPIO_FIXED_LOW>;
>
> The above seems reasonable to me.
>
> Just to throw out an alternative, phandle values of 0 and -1 are
> generally reserved. Historically that means just skip the entry.
> However, you could use that and do something like this:
>
> mode-gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>,
> <&gpio0 1 GPIO_ACTIVE_HIGH>,
> <0>,
> <0xffffffff>;
>
> So 0 means low and ~0 means high. The only advantage I see with it is
That works as long as we don't need other pin states like
high-impedance. I haven't seen anything like that yet though.
> you don't need a "fixed-gpios" driver. Also, I'm not sure how that would
> work with requesting GPIOs given you've essentially defined only 2 GPIO
> lines (high and low). Though Bartosz is doing some work on non-exclusive
> GPIOs.
Could work, or just dynamically allocate one when needed.
>
> Rob
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 8/8] iio: adc: ad4030: Add support for ADAQ4216 and ADAQ4224
2025-09-26 20:37 [PATCH v3 0/8] Add SPI offload support to AD4030 Marcelo Schmitt
` (6 preceding siblings ...)
2025-09-26 20:40 ` [PATCH v3 7/8] dt-bindings: iio: adc: adi,ad4030: Add ADAQ4216 and ADAQ4224 Marcelo Schmitt
@ 2025-09-26 20:41 ` Marcelo Schmitt
2025-09-28 10:26 ` Jonathan Cameron
7 siblings, 1 reply; 23+ messages in thread
From: Marcelo Schmitt @ 2025-09-26 20:41 UTC (permalink / raw)
To: linux-iio, devicetree, linux-doc, linux-spi, linux-kernel
Cc: jic23, michael.hennerich, nuno.sa, eblanc, dlechner, andy, robh,
krzk+dt, conor+dt, corbet, marcelo.schmitt1
ADAQ4216 and ADAQ4224 are similar to AD4030, but feature a PGA circuitry
that scales the analog input signal prior to it reaching the ADC. The PGA
is controlled through a pair of pins (A0 and A1) whose state define the
gain that is applied to the input signal.
Add support for ADAQ4216 and ADAQ4224. Provide a list of PGA options
through the IIO device channel scale available interface and enable control
of the PGA through the channel scale interface.
Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
drivers/iio/adc/ad4030.c | 236 ++++++++++++++++++++++++++++++++++++++-
1 file changed, 232 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/adc/ad4030.c b/drivers/iio/adc/ad4030.c
index 8fca98738e3e..6f1bbf70e153 100644
--- a/drivers/iio/adc/ad4030.c
+++ b/drivers/iio/adc/ad4030.c
@@ -47,6 +47,8 @@
#define AD4030_REG_CHIP_GRADE_AD4630_24_GRADE 0x00
#define AD4030_REG_CHIP_GRADE_AD4632_16_GRADE 0x05
#define AD4030_REG_CHIP_GRADE_AD4632_24_GRADE 0x02
+#define AD4030_REG_CHIP_GRADE_ADAQ4216_GRADE 0x1E
+#define AD4030_REG_CHIP_GRADE_ADAQ4224_GRADE 0x1C
#define AD4030_REG_CHIP_GRADE_MASK_CHIP_GRADE GENMASK(7, 3)
#define AD4030_REG_SCRATCH_PAD 0x0A
#define AD4030_REG_SPI_REVISION 0x0B
@@ -124,6 +126,10 @@
/* Datasheet says 9.8ns, so use the closest integer value */
#define AD4030_TQUIET_CNV_DELAY_NS 10
+/* HARDWARE_GAIN */
+#define ADAQ4616_PGA_PINS 2
+#define ADAQ4616_PGA_GAIN_MAX_NANO (NANO * 2 / 3)
+
enum ad4030_out_mode {
AD4030_OUT_DATA_MD_DIFF,
AD4030_OUT_DATA_MD_16_DIFF_8_COM,
@@ -144,6 +150,30 @@ enum {
AD4030_SCAN_TYPE_AVG,
};
+static const int adaq4216_hw_gains_db[] = {
+ -10, /* 1/3 V/V gain */
+ -5, /* 5/9 V/V gain */
+ 7, /* 20/9 V/V gain */
+ 16, /* 20/3 V/V gain */
+};
+
+/*
+ * Gains computed as fractions of 1000 so they can be expressed by integers.
+ */
+static const int adaq4216_hw_gains_vpv[] = {
+ MILLI / 3, /* 333 */
+ (5 * MILLI / 9), /* 555 */
+ (20 * MILLI / 9), /* 2222 */
+ (20 * MILLI / 3), /* 6666 */
+};
+
+static const int adaq4216_hw_gains_frac[][2] = {
+ { 1, 3 }, /* 1/3 V/V gain */
+ { 5, 9 }, /* 5/9 V/V gain */
+ { 20, 9 }, /* 20/9 V/V gain */
+ { 20, 3 }, /* 20/3 V/V gain */
+};
+
struct ad4030_chip_info {
const char *name;
const unsigned long *available_masks;
@@ -151,6 +181,7 @@ struct ad4030_chip_info {
const struct iio_chan_spec offload_channels[AD4030_MAX_IIO_CHANNEL_NB];
u8 grade;
u8 precision_bits;
+ bool has_pga;
/* Number of hardware channels */
int num_voltage_inputs;
unsigned int tcyc_ns;
@@ -174,7 +205,11 @@ struct ad4030_state {
struct spi_offload_trigger *offload_trigger;
struct spi_offload_trigger_config offload_trigger_config;
struct pwm_device *cnv_trigger;
+ size_t scale_avail_size;
struct pwm_waveform cnv_wf;
+ unsigned int scale_avail[ARRAY_SIZE(adaq4216_hw_gains_db)][2];
+ struct gpio_descs *pga_gpios;
+ unsigned int pga_index;
/*
* DMA (thus cache coherency maintenance) requires the transfer buffers
@@ -231,7 +266,7 @@ struct ad4030_state {
* - voltage0-voltage1
* - voltage2-voltage3
*/
-#define __AD4030_CHAN_DIFF(_idx, _scan_type, _offload) { \
+#define __AD4030_CHAN_DIFF(_idx, _scan_type, _offload, _pga) { \
.info_mask_shared_by_all = \
(_offload ? BIT(IIO_CHAN_INFO_SAMP_FREQ) : 0) | \
BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
@@ -242,6 +277,7 @@ struct ad4030_state {
BIT(IIO_CHAN_INFO_CALIBBIAS) | \
BIT(IIO_CHAN_INFO_RAW), \
.info_mask_separate_available = BIT(IIO_CHAN_INFO_CALIBBIAS) | \
+ (_pga ? BIT(IIO_CHAN_INFO_SCALE) : 0) | \
BIT(IIO_CHAN_INFO_CALIBSCALE), \
.type = IIO_VOLTAGE, \
.indexed = 1, \
@@ -256,10 +292,16 @@ struct ad4030_state {
}
#define AD4030_CHAN_DIFF(_idx, _scan_type) \
- __AD4030_CHAN_DIFF(_idx, _scan_type, 0)
+ __AD4030_CHAN_DIFF(_idx, _scan_type, 0, 0)
#define AD4030_OFFLOAD_CHAN_DIFF(_idx, _scan_type) \
- __AD4030_CHAN_DIFF(_idx, _scan_type, 1)
+ __AD4030_CHAN_DIFF(_idx, _scan_type, 1, 0)
+
+#define ADAQ4216_CHAN_DIFF(_idx, _scan_type) \
+ __AD4030_CHAN_DIFF(_idx, _scan_type, 0, 1)
+
+#define ADAQ4216_OFFLOAD_CHAN_DIFF(_idx, _scan_type) \
+ __AD4030_CHAN_DIFF(_idx, _scan_type, 1, 1)
static const int ad4030_average_modes[] = {
BIT(0), /* No averaging/oversampling */
@@ -413,6 +455,65 @@ static const struct regmap_config ad4030_regmap_config = {
.max_register = AD4030_REG_DIG_ERR,
};
+static void ad4030_fill_scale_avail(struct ad4030_state *st)
+{
+ unsigned int mag_bits, int_part, fract_part, i;
+ u64 range;
+
+ /*
+ * The maximum precision of differential channels is retrieved from the
+ * chip properties. The output code of differential channels is in two's
+ * complement format (i.e. signed), so the MSB is the sign bit and only
+ * (precision_bits - 1) bits express voltage magnitude.
+ */
+ mag_bits = st->chip->precision_bits - 1;
+
+ for (i = 0; i < ARRAY_SIZE(adaq4216_hw_gains_frac); i++) {
+ range = mult_frac(st->vref_uv, adaq4216_hw_gains_frac[i][1],
+ adaq4216_hw_gains_frac[i][0]);
+ /*
+ * If range were in mV, we would multiply it by NANO below.
+ * Though, range is in µV so multiply it by MICRO only so the
+ * result after right shift and division scales output codes to
+ * millivolts.
+ */
+ int_part = div_u64_rem(((u64)range * MICRO) >> mag_bits, NANO, &fract_part);
+ st->scale_avail[i][0] = int_part;
+ st->scale_avail[i][1] = fract_part;
+ }
+}
+
+static int ad4030_set_pga_gain(struct ad4030_state *st)
+{
+ DECLARE_BITMAP(bitmap, ADAQ4616_PGA_PINS) = { };
+
+ bitmap_write(bitmap, st->pga_index, 0, ADAQ4616_PGA_PINS);
+
+ return gpiod_multi_set_value_cansleep(st->pga_gpios, bitmap);
+}
+
+static int ad4030_set_pga(struct iio_dev *indio_dev, int gain_int, int gain_fract)
+{
+ struct ad4030_state *st = iio_priv(indio_dev);
+ unsigned int mag_bits = st->chip->precision_bits - 1;
+ u64 gain_nano, tmp;
+
+ if (!st->pga_gpios)
+ return -EINVAL;
+
+ gain_nano = gain_int * NANO + gain_fract;
+
+ if (!in_range(gain_nano, 1, ADAQ4616_PGA_GAIN_MAX_NANO))
+ return -EINVAL;
+
+ tmp = DIV_ROUND_CLOSEST_ULL(gain_nano << mag_bits, NANO);
+ gain_nano = DIV_ROUND_CLOSEST_ULL(st->vref_uv, tmp);
+ st->pga_index = find_closest(gain_nano, adaq4216_hw_gains_vpv,
+ ARRAY_SIZE(adaq4216_hw_gains_vpv));
+
+ return ad4030_set_pga_gain(st);
+}
+
static int ad4030_get_chan_scale(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
int *val,
@@ -432,7 +533,14 @@ static int ad4030_get_chan_scale(struct iio_dev *indio_dev,
*val2 = scan_type->realbits;
- return IIO_VAL_FRACTIONAL_LOG2;
+ /* The LSB of the 8-bit common-mode data is always vref/256. */
+ if (scan_type->realbits == 8 || !st->chip->has_pga)
+ return IIO_VAL_FRACTIONAL_LOG2;
+
+ *val = st->scale_avail[st->pga_index][0];
+ *val2 = st->scale_avail[st->pga_index][1];
+
+ return IIO_VAL_INT_PLUS_NANO;
}
static int ad4030_get_chan_calibscale(struct iio_dev *indio_dev,
@@ -900,6 +1008,15 @@ static int ad4030_read_avail(struct iio_dev *indio_dev,
*length = ARRAY_SIZE(ad4030_average_modes);
return IIO_AVAIL_LIST;
+ case IIO_CHAN_INFO_SCALE:
+ if (st->scale_avail_size == 1)
+ *vals = (int *)st->scale_avail[st->pga_index];
+ else
+ *vals = (int *)st->scale_avail;
+ *length = st->scale_avail_size * 2; /* print int and nano part */
+ *type = IIO_VAL_INT_PLUS_NANO;
+ return IIO_AVAIL_LIST;
+
default:
return -EINVAL;
}
@@ -975,6 +1092,9 @@ static int ad4030_write_raw_dispatch(struct iio_dev *indio_dev,
case IIO_CHAN_INFO_SAMP_FREQ:
return ad4030_set_sampling_freq(indio_dev, val);
+ case IIO_CHAN_INFO_SCALE:
+ return ad4030_set_pga(indio_dev, val, val2);
+
default:
return -EINVAL;
}
@@ -996,6 +1116,17 @@ static int ad4030_write_raw(struct iio_dev *indio_dev,
return ret;
}
+static int ad4030_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 ad4030_reg_access(struct iio_dev *indio_dev, unsigned int reg,
unsigned int writeval, unsigned int *readval)
{
@@ -1044,6 +1175,7 @@ static const struct iio_info ad4030_iio_info = {
.read_avail = ad4030_read_avail,
.read_raw = ad4030_read_raw,
.write_raw = ad4030_write_raw,
+ .write_raw_get_fmt = &ad4030_write_raw_get_fmt,
.debugfs_reg_access = ad4030_reg_access,
.read_label = ad4030_read_label,
.get_current_scan_type = ad4030_get_current_scan_type,
@@ -1307,6 +1439,50 @@ static int ad4030_spi_offload_setup(struct iio_dev *indio_dev,
IIO_BUFFER_DIRECTION_IN);
}
+static int ad4030_setup_pga(struct device *dev, struct iio_dev *indio_dev,
+ struct ad4030_state *st)
+{
+ unsigned int i;
+ int pga_gain_dB;
+ int ret;
+
+ ret = device_property_read_u32(dev, "adi,pga-gain-db", &pga_gain_dB);
+ if (ret == -EINVAL) {
+ /* Setup GPIOs for PGA control */
+ st->pga_gpios = devm_gpiod_get_array(dev, "pga", GPIOD_OUT_LOW);
+ if (IS_ERR(st->pga_gpios))
+ return dev_err_probe(dev, PTR_ERR(st->pga_gpios),
+ "Failed to get PGA gpios.\n");
+
+ if (st->pga_gpios->ndescs != ADAQ4616_PGA_PINS)
+ return dev_err_probe(dev, -EINVAL,
+ "Expected 2 GPIOs for PGA control.\n");
+
+ st->scale_avail_size = ARRAY_SIZE(adaq4216_hw_gains_db);
+ st->pga_index = 0;
+ return 0;
+ } else if (ret != 0) {
+ return dev_err_probe(dev, ret, "Failed to get PGA value.\n");
+ }
+
+ /* Set ADC driver to handle pin-strapped PGA pins setup */
+ for (i = 0; i < ARRAY_SIZE(adaq4216_hw_gains_db); i++) {
+ if (pga_gain_dB != adaq4216_hw_gains_db[i])
+ continue;
+
+ st->pga_index = i;
+ break;
+ }
+ if (i == ARRAY_SIZE(adaq4216_hw_gains_db))
+ return dev_err_probe(dev, -EINVAL, "Invalid PGA gain: %d.\n",
+ pga_gain_dB);
+
+ st->scale_avail_size = 1;
+ st->pga_gpios = NULL;
+
+ return 0;
+}
+
static int ad4030_probe(struct spi_device *spi)
{
struct device *dev = &spi->dev;
@@ -1349,6 +1525,14 @@ static int ad4030_probe(struct spi_device *spi)
if (ret)
return ret;
+ if (st->chip->has_pga) {
+ ret = ad4030_setup_pga(dev, indio_dev, st);
+ if (ret)
+ return ret;
+
+ ad4030_fill_scale_avail(st);
+ }
+
ret = ad4030_config(st);
if (ret)
return ret;
@@ -1612,12 +1796,54 @@ static const struct ad4030_chip_info ad4632_24_chip_info = {
.max_sample_rate_hz = 500 * HZ_PER_KHZ,
};
+static const struct ad4030_chip_info adaq4216_chip_info = {
+ .name = "adaq4216",
+ .available_masks = ad4030_channel_masks,
+ .channels = {
+ ADAQ4216_CHAN_DIFF(0, ad4030_16_scan_types),
+ AD4030_CHAN_CMO(1, 0),
+ IIO_CHAN_SOFT_TIMESTAMP(2),
+ },
+ .offload_channels = {
+ ADAQ4216_OFFLOAD_CHAN_DIFF(0, ad4030_16_offload_scan_types),
+ AD4030_CHAN_CMO(1, 0),
+ },
+ .grade = AD4030_REG_CHIP_GRADE_ADAQ4216_GRADE,
+ .precision_bits = 16,
+ .has_pga = true,
+ .num_voltage_inputs = 1,
+ .tcyc_ns = AD4030_TCYC_ADJUSTED_NS,
+ .max_sample_rate_hz = 2 * HZ_PER_MHZ,
+};
+
+static const struct ad4030_chip_info adaq4224_chip_info = {
+ .name = "adaq4224",
+ .available_masks = ad4030_channel_masks,
+ .channels = {
+ ADAQ4216_CHAN_DIFF(0, ad4030_24_scan_types),
+ AD4030_CHAN_CMO(1, 0),
+ IIO_CHAN_SOFT_TIMESTAMP(2),
+ },
+ .offload_channels = {
+ ADAQ4216_OFFLOAD_CHAN_DIFF(0, ad4030_24_offload_scan_types),
+ AD4030_CHAN_CMO(1, 0),
+ },
+ .grade = AD4030_REG_CHIP_GRADE_ADAQ4224_GRADE,
+ .precision_bits = 24,
+ .has_pga = true,
+ .num_voltage_inputs = 1,
+ .tcyc_ns = AD4030_TCYC_ADJUSTED_NS,
+ .max_sample_rate_hz = 2 * HZ_PER_MHZ,
+};
+
static const struct spi_device_id ad4030_id_table[] = {
{ "ad4030-24", (kernel_ulong_t)&ad4030_24_chip_info },
{ "ad4630-16", (kernel_ulong_t)&ad4630_16_chip_info },
{ "ad4630-24", (kernel_ulong_t)&ad4630_24_chip_info },
{ "ad4632-16", (kernel_ulong_t)&ad4632_16_chip_info },
{ "ad4632-24", (kernel_ulong_t)&ad4632_24_chip_info },
+ { "adaq4216", (kernel_ulong_t)&adaq4216_chip_info },
+ { "adaq4224", (kernel_ulong_t)&adaq4224_chip_info },
{ }
};
MODULE_DEVICE_TABLE(spi, ad4030_id_table);
@@ -1628,6 +1854,8 @@ static const struct of_device_id ad4030_of_match[] = {
{ .compatible = "adi,ad4630-24", .data = &ad4630_24_chip_info },
{ .compatible = "adi,ad4632-16", .data = &ad4632_16_chip_info },
{ .compatible = "adi,ad4632-24", .data = &ad4632_24_chip_info },
+ { .compatible = "adi,adaq4216", .data = &adaq4216_chip_info },
+ { .compatible = "adi,adaq4224", .data = &adaq4224_chip_info },
{ }
};
MODULE_DEVICE_TABLE(of, ad4030_of_match);
--
2.39.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v3 8/8] iio: adc: ad4030: Add support for ADAQ4216 and ADAQ4224
2025-09-26 20:41 ` [PATCH v3 8/8] iio: adc: ad4030: Add support for " Marcelo Schmitt
@ 2025-09-28 10:26 ` Jonathan Cameron
0 siblings, 0 replies; 23+ messages in thread
From: Jonathan Cameron @ 2025-09-28 10:26 UTC (permalink / raw)
To: Marcelo Schmitt
Cc: linux-iio, devicetree, linux-doc, linux-spi, linux-kernel,
michael.hennerich, nuno.sa, eblanc, dlechner, andy, robh, krzk+dt,
conor+dt, corbet, marcelo.schmitt1
On Fri, 26 Sep 2025 17:41:04 -0300
Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:
> ADAQ4216 and ADAQ4224 are similar to AD4030, but feature a PGA circuitry
> that scales the analog input signal prior to it reaching the ADC. The PGA
> is controlled through a pair of pins (A0 and A1) whose state define the
> gain that is applied to the input signal.
>
> Add support for ADAQ4216 and ADAQ4224. Provide a list of PGA options
> through the IIO device channel scale available interface and enable control
> of the PGA through the channel scale interface.
>
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
One trivial comment + a suggestion that we might want to consider pushing
the pin strap pga support to a future patch so we can not stall the driver
on that being resolved. Note that would mean splitting the DT patch as well.
Anyhow, no rush on that given we have lots of time in this kernel cycle.
Jonathan
> @@ -996,6 +1116,17 @@ static int ad4030_write_raw(struct iio_dev *indio_dev,
> return ret;
> }
> @@ -1307,6 +1439,50 @@ static int ad4030_spi_offload_setup(struct iio_dev *indio_dev,
> IIO_BUFFER_DIRECTION_IN);
> }
>
> +static int ad4030_setup_pga(struct device *dev, struct iio_dev *indio_dev,
> + struct ad4030_state *st)
> +{
> + unsigned int i;
> + int pga_gain_dB;
> + int ret;
> +
> + ret = device_property_read_u32(dev, "adi,pga-gain-db", &pga_gain_dB);
I'm not sure the pga question will resolve quickly so perhaps we initially only
support the pins connected to gpios and add the pin strapped support later?
It's early in this cycle, so we can delay that decision for a few weeks and
see where we are then on that DT question.
> + if (ret == -EINVAL) {
> + /* Setup GPIOs for PGA control */
> + st->pga_gpios = devm_gpiod_get_array(dev, "pga", GPIOD_OUT_LOW);
> + if (IS_ERR(st->pga_gpios))
> + return dev_err_probe(dev, PTR_ERR(st->pga_gpios),
> + "Failed to get PGA gpios.\n");
> +
> + if (st->pga_gpios->ndescs != ADAQ4616_PGA_PINS)
> + return dev_err_probe(dev, -EINVAL,
> + "Expected 2 GPIOs for PGA control.\n");
> +
> + st->scale_avail_size = ARRAY_SIZE(adaq4216_hw_gains_db);
> + st->pga_index = 0;
> + return 0;
> + } else if (ret != 0) {
if (ret) is fairly commonly used for error checking when non 0 is what we want.
> + return dev_err_probe(dev, ret, "Failed to get PGA value.\n");
> + }
> +
> + /* Set ADC driver to handle pin-strapped PGA pins setup */
> + for (i = 0; i < ARRAY_SIZE(adaq4216_hw_gains_db); i++) {
> + if (pga_gain_dB != adaq4216_hw_gains_db[i])
> + continue;
> +
> + st->pga_index = i;
> + break;
> + }
> + if (i == ARRAY_SIZE(adaq4216_hw_gains_db))
> + return dev_err_probe(dev, -EINVAL, "Invalid PGA gain: %d.\n",
> + pga_gain_dB);
> +
> + st->scale_avail_size = 1;
> + st->pga_gpios = NULL;
> +
> + return 0;
> +}
^ permalink raw reply [flat|nested] 23+ messages in thread