iio: adc: ad7768-1: Add features, improvements, and fixes
Subsystems (6)
v5 cover iio: adc: ad7768-1: Add features, improvements, and fixes
This patch series introduces some new features, improvements, and fixes for the AD7768-1 ADC driver.
The goal is to support all key functionalities listed in the device datasheet, including filter mode selection, common mode voltage output configuration and GPIO support. Additionally, this includes fixes for SPI communication and for IIO interface, and also code improvements to enhance maintainability and readability.
---
Changes in v5:
* Added gpio-trigger binding patch.
* Include START pin and DRDY in the trigger-sources description.
* increased trigger-source-cells to 1: this cell will define the trigger
source type.
* Fixed the holes in the regmap ranges.
* replace old iio_device_claim_direct_mode() for the new iio_device_claim/release_direct() functions.
* Changed some commit messages.
* Link to v4: https://lore.kernel.org/linux-iio/[email protected]/T/#t
Changes in v4:
* Added missing select REGMAP_SPI and select REGULATOR to the device's Kconfig.
* VCM output regulator property renamed.
* Added direct mode conditional locks to regulator controller callbacks.
* Renamed regulator controller.
* Created helper function to precalculate the sampling frequency table and avoid race conditions.
* Link to v3: https://lore.kernel.org/linux-iio/[email protected]/T/#t
Changes in v3:
* Fixed irregular or missing SoBs.
* Moved MOSI idle state patch to the start of the patch, as the other fix.
* fixed dt-binding errors.
* Trigger-sources is handled in a different way, as an alternative to sync-in-gpio. (this way we avoid breaking old applications).
* VCM output is controlled by the regulator framework.
* Added a second regmap for 24-bit register values.
* Add new preparatory patch replacing the manual attribute declarations for
the read_avail from struct iio_info.
* included sinc3+rej60 filter type.
* Addressed review comments, see individual pacthes.
* Link to v2: https://lore.kernel.org/linux-iio/[email protected]/T/#u
Changes in v2:
* Removed synchronization over SPI property and replaced it for trigger-sources.
* Added GPIO controller documentation.
* VCM output control changed from an IIO attribute to a devicetree property (static value).
* Converted driver to use regmap and dropped spi_read_reg and spi_write_reg pacthes.
* replaced decimation_rate attribute for oversampling_ratio and dropped device specific documentation patch.
* Added low pass -3dB cutoff attribute.
* Addressed review comments, see individual pacthes.
* Link to v1: https://lore.kernel.org/linux-iio/[email protected]/T/#t
Jonathan Santos (11):
dt-bindings: trigger-source: add generic GPIO trigger source
dt-bindings: iio: adc: ad7768-1: add trigger-sources property
dt-bindings: iio: adc: ad7768-1: Document GPIO controller
dt-bindings: iio: adc: ad7768-1: document regulator provider property
iio: adc: ad7768-1: convert driver to use regmap
iio: adc: ad7768-1: add regulator to control VCM output
iio: adc: ad7768-1: add multiple scan types to support 16-bits mode
iio: adc: ad7768-1: add support for Synchronization over SPI
iio: adc: ad7768-1: replace manual attribute declaration
iio: adc: ad7768-1: add filter type and oversampling ratio attributes
iio: adc: ad7768-1: add low pass -3dB cutoff attribute
Sergiu Cuciurean (3):
iio: adc: ad7768-1: Add reset gpio
iio: adc: ad7768-1: Move buffer allocation to a separate function
iio: adc: ad7768-1: Add GPIO controller support
.../bindings/iio/adc/adi,ad7768-1.yaml | 67 +- .../bindings/trigger-source/gpio-trigger.yaml | 40 + drivers/iio/adc/Kconfig | 2 + drivers/iio/adc/ad7768-1.c | 1105 ++++++++++++++--- 4 files changed, 1040 insertions(+), 174 deletions(-) create mode 100644 Documentation/devicetree/bindings/trigger-source/gpio-trigger.yaml
base-commit: 5d1a5c4f121f0ec50327e899c9450978505f1560
prerequisite-patch-id: 933ca5331b0044084f60cba5f9019663e01fa9c2
Signature · 1 line
2.34.1
PATCH v5 1/14 dt-bindings: trigger-source: add generic GPIO trigger source
Inspired by pwn-trigger, create a new binding for using a GPIO pin as a trigger source.
---
v5 Changes:
* New patch in v5.
--- .../bindings/trigger-source/gpio-trigger.yaml | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 Documentation/devicetree/bindings/trigger-source/gpio-trigger.yaml
Documentation/devicetree/bindings/trigger-source/gpio-trigger.yaml new file +40 -0
@@ -0,0 +1,40 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/trigger-source/gpio-trigger.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Generic trigger source using GPIO + +description: Remaps a GPIO pin as a trigger source. + +maintainers: + - Jonathan Santos <[email protected]> + +properties: + compatible: + const: gpio-trigger + + '#trigger-source-cells': + const: 0 + + gpios: + maxItems: 1 + description: GPIO to be used as a trigger source. + +required: + - compatible + - '#trigger-source-cells' + - gpios + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + + trigger { + compatible = "gpio-trigger"; + #trigger-source-cells = <0>; + gpios = <&gpio1 15 GPIO_ACTIVE_HIGH>; + };
Signature · 1 line
2.34.1
Re: [PATCH v5 01/14] dt-bindings: trigger-source: add generic GPIO trigger source
Quoted 18 lines · Jonathan Santos
Inspired by pwn-trigger, create a new binding for using a GPIO pin as a trigger source.
Signed-off-by Jonathan Santos <[email protected]>---
v5 Changes:
* New patch in v5.--- .../bindings/trigger-source/gpio-trigger.yaml | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 Documentation/devicetree/bindings/trigger-source/gpio-trigger.yamlDocumentation/devicetree/bindings/trigger-source/gpio-trigger.yaml new file +0
-0
Documentation/devicetree/bindings/trigger-source/gpio-trigger.yaml line 9
Quoted 10 lines
Documentation/devicetree/bindings/trigger-source/gpio-trigger.yaml new file +9
-0@@ -0,0 +1,40 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/trigger-source/gpio-trigger.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Generic trigger source using GPIO + +description: Remaps a GPIO pin as a trigger source.
Just a minor comment about the description, "remaps" is a very software term, I think you can just say "A GPIO used as a trigger source" and be fine.
Quoted 34 lines
Documentation/devicetree/bindings/trigger-source/gpio-trigger.yaml new file +31
-0+ +maintainers: + - Jonathan Santos <[email protected]> + +properties: + compatible: + const: gpio-trigger + + '#trigger-source-cells': + const: 0 + + gpios: + maxItems: 1 + description: GPIO to be used as a trigger source. + +required: + - compatible + - '#trigger-source-cells' + - gpios + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + + trigger { + compatible = "gpio-trigger"; + #trigger-source-cells = <0>; + gpios = <&gpio1 15 GPIO_ACTIVE_HIGH>; + };Signature · 1 line
2.34.1
David Lechner
maintainerRe: [PATCH v5 01/14] dt-bindings: trigger-source: add generic GPIO trigger source
In reply to Jonathan SantosQuoted 1 line · Jonathan Santos
Inspired by pwn-trigger, create a new binding for using a GPIO
s/pwn/pwm/
Quoted 2 lines
pin as a trigger source.
Re: [PATCH v5 01/14] dt-bindings: trigger-source: add generic GPIO trigger source
In reply to Jonathan SantosHi Jonathan,
thanks for your patch!
Quoted 4 lines · Jonathan Santos
Inspired by pwn-trigger, create a new binding for using a GPIO pin as a trigger source.
Signed-off-by Jonathan Santos <[email protected]>
Is this actually documenting the trigger sources I implemented for LED here?
https://lore.kernel.org/all/[email protected]/
Then maybe put this in as Link:
I tried to figure out how to properly document it but I think it was part of dtsschema and that may have confused me.
Documentation/devicetree/bindings/trigger-source/gpio-trigger.yaml line 9
Quoted 3 lines
Documentation/devicetree/bindings/trigger-source/gpio-trigger.yaml new file +3
-0+title: Generic trigger source using GPIO + +description: Remaps a GPIO pin as a trigger source.
Please write "GPIO line" instead of "GPIO pin".
The reason is that not all GPIOs are pins. Some are other stuff.
Yours,
Linus Walleij
David Lechner
maintainerRe: [PATCH v5 01/14] dt-bindings: trigger-source: add generic GPIO trigger source
Quoted 7 lines · Linus Walleij
Hi Jonathan,
thanks for your patch!
On Fri, Apr 11, 2025 at 5:56 PM Jonathan Santos
<[email protected]> wrote:
Quoted 8 lines
Quoted 4 lines
Inspired by pwn-trigger, create a new binding for using a GPIO pin as a trigger source.
Signed-off-by Jonathan Santos <[email protected]>Is this actually documenting the trigger sources I implemented for LED here?
https://lore.kernel.org/all/[email protected]/
No. This is something more like "pwm-clock" where it is converting a gpios phandle to a trigger-sources phandle. Doing it this way comes from some discussion on using trigger-sources for the SPI offload stuff I was working on recently. [1]
As a result of that work, there are generic bindings for trigger-sources and #trigger-source cells in dtschma now that can be used by any node. [2]
The way the leds subsystem does it where you can have trigger-sources = <&gpio 0> or <&ohci_port1> directly has the issue where it isn't easy for the consumer to know what type of node the provider is. Effectively, we have to have the linux,default-trigger property to tell us what the provider node type is. By adding this extra node in between we can get that type info more naturally than the linux-specific property. And each subsystem won't have to have extra code added for trigger-sources like you had to add for gpios.
[1]: https://lore.kernel.org/all/20241031-croon-boss-3b30ff9e9333@spud/
[2]: https://github.com/devicetree-org/dt-schema/commit/93ee8008959e362548168eaa7bdc20c115f3d586
PATCH v5 2/14 dt-bindings: iio: adc: ad7768-1: add trigger-sources property
In reply to Jonathan SantosIn addition to GPIO synchronization, The AD7768-1 also supports synchronization over SPI, which use is recommended when the GPIO cannot provide a pulse synchronous with the base MCLK signal. It consists of looping back the SYNC_OUT to the SYNC_IN pin and send a command via SPI to trigger the synchronization.
Introduce the 'trigger-sources' property to support SPI-based synchronization, along with additional optional entries for the SPI offload trigger and the START signal via GPIO3.
While at it, add description to the interrupts property.
---
v5 Changes:
* Include START pin and DRDY in the trigger-sources description.
* Fixed "#trigger-source-cells" value and description.
* sync-in-gpios is represented in the trigger-sources property.
v4 Changes:
* none
v3 Changes:
* Fixed dt-bindings errors.
* Trigger-source is set as an alternative to sync-in-gpios, so we don't break the previous ABI.
* increased maxItems from trigger-sources to 2.
v2 Changes:
* Patch added as replacement for adi,sync-in-spi patch.
* addressed the request for a description to interrupts property.
--- .../bindings/iio/adc/adi,ad7768-1.yaml | 38 +++++++++++++++++-- 1 file changed, 35 insertions(+), 3 deletions(-)
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +35 -3
@@ -26,7 +26,30 @@ properties: clock-names: const: mclk + trigger-sources: + $ref: /schemas/types.yaml#/definitions/phandle-array + minItems: 1 + maxItems: 3 + description: | + A list of phandles referencing trigger source devices or GPIOs. + Supports up to three entries, each representing a different type of + trigger: + + - First entry specifies the device responsible for driving the + synchronization (SYNC_IN) pin, as an alternative to adi,sync-in-gpios. + This can be a `gpio-trigger` or another `ad7768-1` device. If the + device's own SYNC_OUT pin is internally connected to its SYNC_IN pin, + reference the device itself or omit this property. + - Second entry optionally defines a GPIO3 pin used as a START signal trigger. + - Third entry specifies a GPIO line to act as a trigger for SPI offload. + + Use the accompanying trigger source cell to identify the type of each entry. + interrupts: + description: + Specifies the interrupt line associated with the ADC. This refers + to the DRDY (Data Ready) pin, which signals when conversion results are + available. maxItems: 1 '#address-cells':
@@ -57,6 +80,15 @@ properties: "#io-channel-cells": const: 1 + "#trigger-source-cells": + description: | + Indicates the trigger source type for each entry: + 0 = Synchronization GPIO-based trigger + 1 = Synchronization device trigger (e.g., another ad7768-1) + 2 = GPIO3 pin acting as START signal + 3 = DRDY pin acting as SPI offload trigger + const: 1 + required: - compatible - reg
@@ -65,7 +97,6 @@ required: - vref-supply - spi-cpol - spi-cpha - - adi,sync-in-gpios patternProperties: "^channel@([0-9]|1[0-5])$":
@@ -99,7 +130,7 @@ examples: #address-cells = <1>; #size-cells = <0>; - adc@0 { + adc0: adc@0 { compatible = "adi,ad7768-1"; reg = <0>; spi-max-frequency = <2000000>;
@@ -108,7 +139,8 @@ examples: vref-supply = <&adc_vref>; interrupts = <25 IRQ_TYPE_EDGE_RISING>; interrupt-parent = <&gpio>; - adi,sync-in-gpios = <&gpio 22 GPIO_ACTIVE_LOW>; + trigger-sources = <&adc0 1>; + #trigger-source-cells = <1>; reset-gpios = <&gpio 27 GPIO_ACTIVE_LOW>; clocks = <&ad7768_mclk>; clock-names = "mclk";
Signature · 1 line
2.34.1
Re: [PATCH v5 02/14] dt-bindings: iio: adc: ad7768-1: add trigger-sources property
Quoted 71 lines · Jonathan Santos
In addition to GPIO synchronization, The AD7768-1 also supports synchronization over SPI, which use is recommended when the GPIO cannot provide a pulse synchronous with the base MCLK signal. It consists of looping back the
SYNC_OUTto theSYNC_INpin and send a command via SPI to trigger the synchronization.Introduce the 'trigger-sources' property to support SPI-based synchronization, along with additional optional entries for the SPI offload trigger and the START signal via GPIO3.
While at it, add description to the interrupts property.
Signed-off-by Jonathan Santos <[email protected]>---
v5 Changes:
* Include START pin and DRDY in the trigger-sources description.
* Fixed "#trigger-source-cells" value and description.
* sync-in-gpios is represented in the trigger-sources property.v4 Changes:
* nonev3 Changes:
* Fixed dt-bindings errors.
* Trigger-source is set as an alternative to sync-in-gpios, so we don't break the previous ABI.
* increased maxItems from trigger-sources to 2.v2 Changes:
* Patch added as replacement for adi,sync-in-spi patch.
* addressed the request for a description to interrupts property.--- .../bindings/iio/adc/adi,ad7768-1.yaml | 38 +++++++++++++++++-- 1 file changed, 35 insertions(+), 3 deletions(-)Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +23
-0@@ -26,7 +26,30 @@ properties: clock-names: const: mclk + trigger-sources: + $ref: /schemas/types.yaml#/definitions/phandle-array + minItems: 1 + maxItems: 3 + description: | + A list of phandles referencing trigger source devices or GPIOs. + Supports up to three entries, each representing a different type of + trigger: + + - First entry specifies the device responsible for driving the + synchronization (SYNC_IN) pin, as an alternative to adi,sync-in-gpios. + This can be a `gpio-trigger` or another `ad7768-1` device. If the + device's own SYNC_OUT pin is internally connected to its SYNC_IN pin, + reference the device itself or omit this property. + - Second entry optionally defines a GPIO3 pin used as a START signal trigger. + - Third entry specifies a GPIO line to act as a trigger for SPI offload. + + Use the accompanying trigger source cell to identify the type of each entry. + interrupts: + description: + Specifies the interrupt line associated with the ADC. This refers + to the DRDY (Data Ready) pin, which signals when conversion results are + available. maxItems: 1 '#address-cells':
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml properties: · line 89
Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +7
-0@@ -57,6 +80,15 @@ properties: "#io-channel-cells": const: 1 + "#trigger-source-cells": + description: | + Indicates the trigger source type for each entry: + 0 = Synchronization GPIO-based trigger + 1 = Synchronization device trigger (e.g., another ad7768-1) + 2 = GPIO3 pin acting as START signal + 3 = DRDY pin acting as SPI offload trigger
I think the description here makes little sense, when the value is required to be 1 by the following line. If these are permitted values for things consuming this adc as a trigger source, you should specify that IMO.
David Lechner
maintainerRe: [PATCH v5 02/14] dt-bindings: iio: adc: ad7768-1: add trigger-sources property
In reply to Jonathan SantosQuoted 40 lines · Jonathan Santos
In addition to GPIO synchronization, The AD7768-1 also supports synchronization over SPI, which use is recommended when the GPIO cannot provide a pulse synchronous with the base MCLK signal. It consists of looping back the
SYNC_OUTto theSYNC_INpin and send a command via SPI to trigger the synchronization.Introduce the 'trigger-sources' property to support SPI-based synchronization, along with additional optional entries for the SPI offload trigger and the START signal via GPIO3.
While at it, add description to the interrupts property.
Signed-off-by Jonathan Santos <[email protected]>---
v5 Changes:
* Include START pin and DRDY in the trigger-sources description.
* Fixed "#trigger-source-cells" value and description.
* sync-in-gpios is represented in the trigger-sources property.v4 Changes:
* nonev3 Changes:
* Fixed dt-bindings errors.
* Trigger-source is set as an alternative to sync-in-gpios, so we don't break the previous ABI.
* increased maxItems from trigger-sources to 2.v2 Changes:
* Patch added as replacement for adi,sync-in-spi patch.
* addressed the request for a description to interrupts property.--- .../bindings/iio/adc/adi,ad7768-1.yaml | 38 +++++++++++++++++-- 1 file changed, 35 insertions(+), 3 deletions(-)Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +0
-0
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml properties: · line 34
Quoted 9 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +6
-0@@ -26,7 +26,30 @@ properties: clock-names: const: mclk + trigger-sources: + $ref: /schemas/types.yaml#/definitions/phandle-array + minItems: 1 + maxItems: 3 + description: | + A list of phandles referencing trigger source devices or GPIOs.
I don't think a gpio phandle should be directly allowed. Only a trigger source provider (something with #trigger-source-cells).
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml properties: · line 44
Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +10
-0+ Supports up to three entries, each representing a different type of + trigger: + + - First entry specifies the device responsible for driving the + synchronization (SYNC_IN) pin, as an alternative to adi,sync-in-gpios. + This can be a `gpio-trigger` or another `ad7768-1` device. If the + device's own SYNC_OUT pin is internally connected to its SYNC_IN pin, + reference the device itself or omit this property. + - Second entry optionally defines a GPIO3 pin used as a START signal trigger. + - Third entry specifies a GPIO line to act as a trigger for SPI offload.
SPI offload is part of the SPI controller, not the ADC chip, so doesn't make sense to have that binding here. In that case, the ADC is the trigger-source provider, not consumer.
Re: [PATCH v5 02/14] dt-bindings: iio: adc: ad7768-1: add trigger-sources property
Quoted 44 lines · David Lechner
Quoted 43 lines · Jonathan Santos
In addition to GPIO synchronization, The AD7768-1 also supports synchronization over SPI, which use is recommended when the GPIO cannot provide a pulse synchronous with the base MCLK signal. It consists of looping back the
SYNC_OUTto theSYNC_INpin and send a command via SPI to trigger the synchronization.Introduce the 'trigger-sources' property to support SPI-based synchronization, along with additional optional entries for the SPI offload trigger and the START signal via GPIO3.
While at it, add description to the interrupts property.
Signed-off-by Jonathan Santos <[email protected]>---
v5 Changes:
* Include START pin and DRDY in the trigger-sources description.
* Fixed "#trigger-source-cells" value and description.
* sync-in-gpios is represented in the trigger-sources property.v4 Changes:
* nonev3 Changes:
* Fixed dt-bindings errors.
* Trigger-source is set as an alternative to sync-in-gpios, so we don't break the previous ABI.
* increased maxItems from trigger-sources to 2.v2 Changes:
* Patch added as replacement for adi,sync-in-spi patch.
* addressed the request for a description to interrupts property.--- .../bindings/iio/adc/adi,ad7768-1.yaml | 38 +++++++++++++++++-- 1 file changed, 35 insertions(+), 3 deletions(-)Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +0
-0@@ -26,7 +26,30 @@ properties: clock-names: const: mclk
Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml properties: · line 34
Quoted 6 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +6
-0+ trigger-sources: + $ref: /schemas/types.yaml#/definitions/phandle-array + minItems: 1 + maxItems: 3 + description: | + A list of phandles referencing trigger source devices or GPIOs.I don't think a gpio phandle should be directly allowed. Only a trigger source provider (something with #trigger-source-cells).
Sorry, I meant gpio-trigger, but I phrased it incorrectly.
Quoted 11 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml properties: · line 44
Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +10
-0+ Supports up to three entries, each representing a different type of + trigger: + + - First entry specifies the device responsible for driving the + synchronization (SYNC_IN) pin, as an alternative to adi,sync-in-gpios. + This can be a `gpio-trigger` or another `ad7768-1` device. If the + device's own SYNC_OUT pin is internally connected to its SYNC_IN pin, + reference the device itself or omit this property. + - Second entry optionally defines a GPIO3 pin used as a START signal trigger. + - Third entry specifies a GPIO line to act as a trigger for SPI offload.
Quoted 3 lines
SPI offload is part of the SPI controller, not the ADC chip, so doesn't make sense to have that binding here. In that case, the ADC is the trigger-source provider, not consumer.
Right! Maybe a silly question, but this means we would have then two trigger-sources defined, one in the spi controller node and other in the adc node, right? like this:
spi_controller: spi@44a00000 {
...
trigger-sources = <&offload_trigger_source>;
...
adc0@ {
...
trigger-sources = <&sync_trigger_source>;
#trigger-source-cells = <1>;
...
}
}Quoted 3 lines
David Lechner
maintainerRe: [PATCH v5 02/14] dt-bindings: iio: adc: ad7768-1: add trigger-sources property
Quoted 16 lines · Jonathan Santos
Quoted 15 lines · David Lechner
Quoted 14 lines · Jonathan Santos
In addition to GPIO synchronization, The AD7768-1 also supports synchronization over SPI, which use is recommended when the GPIO cannot provide a pulse synchronous with the base MCLK signal. It consists of looping back the
SYNC_OUTto theSYNC_INpin and send a command via SPI to trigger the synchronization.Introduce the 'trigger-sources' property to support SPI-based synchronization, along with additional optional entries for the SPI offload trigger and the START signal via GPIO3.
While at it, add description to the interrupts property.
Signed-off-by Jonathan Santos <[email protected]>---
...
Quoted 19 lines
Quoted 11 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml properties: · line 44
Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +10
-0+ Supports up to three entries, each representing a different type of + trigger: + + - First entry specifies the device responsible for driving the + synchronization (SYNC_IN) pin, as an alternative to adi,sync-in-gpios. + This can be a `gpio-trigger` or another `ad7768-1` device. If the + device's own SYNC_OUT pin is internally connected to its SYNC_IN pin, + reference the device itself or omit this property. + - Second entry optionally defines a GPIO3 pin used as a START signal trigger. + - Third entry specifies a GPIO line to act as a trigger for SPI offload.Quoted 3 lines
SPI offload is part of the SPI controller, not the ADC chip, so doesn't make sense to have that binding here. In that case, the ADC is the trigger-source provider, not consumer.
Right! Maybe a silly question, but this means we would have then two trigger-sources defined, one in the spi controller node and other in the adc node, right? like this:
Quoted 11 lines
spi_controller: spi@44a00000 {
...
trigger-sources = <&offload_trigger_source>;
...
adc0@ {
...
trigger-sources = <&sync_trigger_source>;
#trigger-source-cells = <1>;
...
}
}Yes, this looks correct. And for the case of SYNC_OUT connected to SYNC_IN on the ADC itself, we could omit trigger-sources = <&sync_trigger_source>;.
David Lechner
maintainerRe: [PATCH v5 02/14] dt-bindings: iio: adc: ad7768-1: add trigger-sources property
In reply to Jonathan SantosQuoted 14 lines · Jonathan Santos
In addition to GPIO synchronization, The AD7768-1 also supports synchronization over SPI, which use is recommended when the GPIO cannot provide a pulse synchronous with the base MCLK signal. It consists of looping back the
SYNC_OUTto theSYNC_INpin and send a command via SPI to trigger the synchronization.Introduce the 'trigger-sources' property to support SPI-based synchronization, along with additional optional entries for the SPI offload trigger and the START signal via GPIO3.
While at it, add description to the interrupts property.
Signed-off-by Jonathan Santos <[email protected]>---
...
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml properties: · line 91
Quoted 13 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +9
-0@@ -57,6 +80,15 @@ properties: "#io-channel-cells": const: 1 + "#trigger-source-cells": + description: | + Indicates the trigger source type for each entry: + 0 = Synchronization GPIO-based trigger + 1 = Synchronization device trigger (e.g., another ad7768-1) + 2 = GPIO3 pin acting as START signal + 3 = DRDY pin acting as SPI offload trigger + const: 1 +
0 and 1 don't sound like trigger outputs that this ADC is providing, so don't seem appropriate here. But the SYNC_OUT pin is missing from this list.
Also, outputs could be used to trigger anything, not just SPI offload, so don't need to mention that.
Re: [PATCH v5 02/14] dt-bindings: iio: adc: ad7768-1: add trigger-sources property
Quoted 35 lines · David Lechner
Quoted 14 lines · Jonathan Santos
In addition to GPIO synchronization, The AD7768-1 also supports synchronization over SPI, which use is recommended when the GPIO cannot provide a pulse synchronous with the base MCLK signal. It consists of looping back the
SYNC_OUTto theSYNC_INpin and send a command via SPI to trigger the synchronization.Introduce the 'trigger-sources' property to support SPI-based synchronization, along with additional optional entries for the SPI offload trigger and the START signal via GPIO3.
While at it, add description to the interrupts property.
Signed-off-by Jonathan Santos <[email protected]>---
...
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml properties: · line 91
Quoted 13 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +9
-0@@ -57,6 +80,15 @@ properties: "#io-channel-cells": const: 1 + "#trigger-source-cells": + description: | + Indicates the trigger source type for each entry: + 0 = Synchronization GPIO-based trigger + 1 = Synchronization device trigger (e.g., another ad7768-1) + 2 = GPIO3 pin acting as START signal + 3 = DRDY pin acting as SPI offload trigger + const: 1 +0 and 1 don't sound like trigger outputs that this ADC is providing, so don't seem appropriate here. But the
SYNC_OUTpin is missing from this list.
Quoted 2 lines
Also, outputs could be used to trigger anything, not just SPI offload, so don't need to mention that.
You mean like this:
...
"#trigger-source-cells":
description: |
Cell indicates the trigger output signal: 0 = SYNC_OUT, 1 = GPIO3, 2 = DRDY.
const: 1
...
It would be like interfacing those output pins for a generic trigger usage?
Quoted 1 line
David Lechner
maintainerRe: [PATCH v5 02/14] dt-bindings: iio: adc: ad7768-1: add trigger-sources property
Quoted 47 lines · Jonathan Santos
Quoted 35 lines · David Lechner
Quoted 14 lines · Jonathan Santos
In addition to GPIO synchronization, The AD7768-1 also supports synchronization over SPI, which use is recommended when the GPIO cannot provide a pulse synchronous with the base MCLK signal. It consists of looping back the
SYNC_OUTto theSYNC_INpin and send a command via SPI to trigger the synchronization.Introduce the 'trigger-sources' property to support SPI-based synchronization, along with additional optional entries for the SPI offload trigger and the START signal via GPIO3.
While at it, add description to the interrupts property.
Signed-off-by Jonathan Santos <[email protected]>---
...
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml properties: · line 91
Quoted 13 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +9
-0@@ -57,6 +80,15 @@ properties: "#io-channel-cells": const: 1 + "#trigger-source-cells": + description: | + Indicates the trigger source type for each entry: + 0 = Synchronization GPIO-based trigger + 1 = Synchronization device trigger (e.g., another ad7768-1) + 2 = GPIO3 pin acting as START signal + 3 = DRDY pin acting as SPI offload trigger + const: 1 +0 and 1 don't sound like trigger outputs that this ADC is providing, so don't seem appropriate here. But the
SYNC_OUTpin is missing from this list.Quoted 2 lines
Also, outputs could be used to trigger anything, not just SPI offload, so don't need to mention that.
You mean like this:
...
"#trigger-source-cells":
description: |
Cell indicates the trigger output signal: 0 =SYNC_OUT, 1 = GPIO3, 2 = DRDY.
Quoted 7 lines
const: 1
...It would be like interfacing those output pins for a generic trigger usage?
Quoted 1 line
Yes this looks correct now.
I don't think this is the case, but in general, if GPIO3 could be programmed to have different trigger signals, then we would need a 2nd cell. But IIRC, it can only be the START signal, so 1 cell should be sufficient in this case.
PATCH v5 3/14 dt-bindings: iio: adc: ad7768-1: Document GPIO controller
In reply to Jonathan SantosThe AD7768-1 ADC exports four bidirectional GPIOs accessible via register map.
Document GPIO properties necessary to enable GPIO controller for this device.
---
v5 Changes:
* none.
v4 Changes:
* none.
v3 Changes:
* none.
v2 Changes:
* New patch in v2.
--- .../devicetree/bindings/iio/adc/adi,ad7768-1.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+)
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +10 -0
@@ -89,6 +89,14 @@ properties: 3 = DRDY pin acting as SPI offload trigger const: 1 + gpio-controller: true + + "#gpio-cells": + const: 2 + description: | + The first cell is for the GPIO number: 0 to 3. + The second cell takes standard GPIO flags. + required: - compatible - reg
@@ -136,6 +144,8 @@ examples: spi-max-frequency = <2000000>; spi-cpol; spi-cpha; + gpio-controller; + #gpio-cells = <2>; vref-supply = <&adc_vref>; interrupts = <25 IRQ_TYPE_EDGE_RISING>; interrupt-parent = <&gpio>;
Signature · 1 line
2.34.1
PATCH v5 4/14 dt-bindings: iio: adc: ad7768-1: document regulator provider property
In reply to Jonathan SantosThe AD7768-1 provides a buffered common-mode voltage output on the VCM pin that can be used to bias analog input signals.
Add regulators property to enable the use of the VCM output, referenced here as vcm-output, by any other device.
---
v5 Changes:
* removed regulator-min-microvolt and regulator-max-microvolt.
v4 Changes:
* replace "vcm_output" property name for "vcm-output".
v3 Changes:
* VCM is now provided as a regulator within the device, instead of a custom property.
v2 Changes:
* New patch in v2.
--- .../bindings/iio/adc/adi,ad7768-1.yaml | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +19 -0
@@ -70,6 +70,19 @@ properties: in any way, for example if the filter decimation rate changes. As the line is active low, it should be marked GPIO_ACTIVE_LOW. + regulators: + type: object + description: + list of regulators provided by this controller. + + properties: + vcm-output: + $ref: /schemas/regulator/regulator.yaml# + type: object + unevaluatedProperties: false + + additionalProperties: false + reset-gpios: maxItems: 1
@@ -162,6 +175,12 @@ examples: reg = <0>; label = "channel_0"; }; + + regulators { + vcm_reg: vcm-output { + regulator-name = "ad7768-1-vcm"; + }; + }; }; }; ...
Signature · 1 line
2.34.1
PATCH v5 5/14 iio: adc: ad7768-1: convert driver to use regmap
In reply to Jonathan SantosConvert the AD7768-1 driver to use the regmap API for register access. This change simplifies and standardizes register interactions, reducing code duplication and improving maintainability.
Create two regmap configurations, one for 8-bit register values and other for 24-bit register values.
Since we are using regmap now, define the remaining registers from 0x32 to 0x34.
---
v5 Changes:
* fixed the holes in the register ranges.
* Removed AD7768_RD_FLAG_MSK and AD7768_WR_FLAG_MSK macros
v4 Changes:
* Add REGMAP24 to the register macros with 24-bit value.
* Add select REGMAP_SPI line to the Kconfig.
v3 Changes:
* Included a second register map for the 24-bit register values.
* Added register tables to separate the 24-bit from the 8-bit values.
v2 Changes:
* New patch in v2.
--- drivers/iio/adc/Kconfig | 1 + drivers/iio/adc/ad7768-1.c | 160 +++++++++++++++++++++++++------------ 2 files changed, 110 insertions(+), 51 deletions(-)
drivers/iio/adc/Kconfig +1 -0
@@ -322,6 +322,7 @@ config AD7766 config AD7768_1 tristate "Analog Devices AD7768-1 ADC driver" depends on SPI + select REGMAP_SPI select IIO_BUFFER select IIO_TRIGGER select IIO_TRIGGERED_BUFFER
drivers/iio/adc/ad7768-1.c +109 -51
262 lines not on this page · raw message
Signature · 1 line
2.34.1
Jonathan Cameron
maintainerRe: [PATCH v5 05/14] iio: adc: ad7768-1: convert driver to use regmap
Quoted 4 lines · On Fri, 11 Apr 2025 12:56:56 -0300 Jonathan Santos…
Convert the AD7768-1 driver to use the regmap API for register access. This change simplifies and standardizes register interactions, reducing code duplication and improving maintainability.
Quoted 8 lines
Create two regmap configurations, one for 8-bit register values and other for 24-bit register values.
Since we are using regmap now, define the remaining registers from
0x32to0x34.Reviewed-by David Lechner <[email protected]>Signed-off-by Jonathan Santos <[email protected]>
Hi Jonathan,
Given this is a fairly substantial set and some parts stand on their own, I'm going to pick up some patches now despite discussion being ongoing about bindings etc. Please rebase your next version on the testing branch of iio.git to take this into account.
Applied this patch.
Thanks,
Jonathan
PATCH v5 6/14 iio: adc: ad7768-1: Add reset gpio
In reply to Jonathan SantosFrom: Sergiu Cuciurean <[email protected]>
Implement asynchronous hardware reset GPIO.
---
v5 Changes:
* simplified commit message.
v4 Changes:
* None.
v3 Changes:
* fixed SoB order.
* increased delay after finishing the reset action to 200us, as the datasheet recommends.
v2 Changes:
* Replaced usleep_range() for fsleep() and gpiod_direction_output() for gpiod_set_value_cansleep().
* Reset via SPI register is performed if the Reset GPIO is not defined.
--- drivers/iio/adc/ad7768-1.c | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-)
drivers/iio/adc/ad7768-1.c +24 -12
@@ -163,6 +163,7 @@ struct ad7768_state {struct completion completion;struct iio_trigger *trig;struct gpio_desc *gpio_sync_in; +struct gpio_desc *gpio_reset;const char *labels[ARRAY_SIZE (ad7768_channels)];/* * DMA (thus cache coherency maintenance) may require the
@@ -487,19 +488,30 @@ static int ad7768_setup(struct ad7768_state *st) {int ret; -/* -* Two writes to the SPI_RESET[1:0] bits are required to initiate -* a software reset. The bits must first be set to 11, and then -* to 10. When the sequence is detected, the reset occurs. -* See the datasheet, page 70. -*/ - ret =regmap_write (st->regmap,AD7768_REG_SYNC_RESET ,0x3 ); -if (ret) -return ret; + st->gpio_reset =devm_gpiod_get_optional (&st->spi->dev,"reset" , +GPIOD_OUT_HIGH ); +if (IS_ERR (st->gpio_reset)) +return PTR_ERR (st->gpio_reset); - ret =regmap_write (st->regmap,AD7768_REG_SYNC_RESET ,0x2 ); -if (ret) -return ret; +if (st->gpio_reset) { +fsleep (10 ); +gpiod_set_value_cansleep (st->gpio_reset,0 ); +fsleep (200 ); + }else { +/* +* Two writes to the SPI_RESET[1:0] bits are required to initiate +* a software reset. The bits must first be set to 11, and then +* to 10. When the sequence is detected, the reset occurs. +* See the datasheet, page 70. +*/ + ret =regmap_write (st->regmap,AD7768_REG_SYNC_RESET ,0x3 ); +if (ret) +return ret; + + ret =regmap_write (st->regmap,AD7768_REG_SYNC_RESET ,0x2 ); +if (ret) +return ret; + } st->gpio_sync_in =devm_gpiod_get (&st->spi->dev,"adi,sync-in" ,GPIOD_OUT_LOW );
Signature · 1 line
2.34.1
Jonathan Cameron
maintainerRe: [PATCH v5 06/14] iio: adc: ad7768-1: Add reset gpio
Quoted 9 lines · On Fri, 11 Apr 2025 12:57:09 -0300 Jonathan Santos…
From: Sergiu Cuciurean <[email protected]>
Implement asynchronous hardware reset GPIO.
Reviewed-by David Lechner <[email protected]>Reviewed-by Marcelo Schmitt <[email protected]>Signed-off-by Sergiu Cuciurean <[email protected]>Co-developed-by Jonathan Santos <[email protected]>Signed-off-by Jonathan Santos <[email protected]>
Applied to the togreg branch of iio.git and pushed out as testing.
Thanks,
Jonathan
PATCH v5 7/14 iio: adc: ad7768-1: Move buffer allocation to a separate function
In reply to Jonathan SantosFrom: Sergiu Cuciurean <[email protected]>
This change moves the buffer allocation and related trigger allocation in a separate function, making space for adding another type of iio buffer if needed.
---
v5 Changes:
* mentioned trigger allocation in the commit message.
v4 Changes:
* None.
v3 Changes:
* Added missing SoB.
v2 Changes:
* Interrupt and completion moved out from ad7768_triggered_buffer_alloc().
--- drivers/iio/adc/ad7768-1.c | 44 ++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 18 deletions(-)
drivers/iio/adc/ad7768-1.c +26 -18
@@ -619,6 +619,31 @@ static int ad7768_set_channel_label(struct iio_dev *indio_dev,return 0 ; } +static int ad7768_triggered_buffer_alloc (struct iio_dev *indio_dev) +{ +struct ad7768_state *st =iio_priv (indio_dev); +int ret; + + st->trig =devm_iio_trigger_alloc (indio_dev->dev.parent,"%s-dev%d" , + indio_dev->name, +iio_device_id (indio_dev)); +if (!st->trig) +return -ENOMEM ; + + st->trig->ops = &ad7768_trigger_ops; +iio_trigger_set_drvdata (st->trig, indio_dev); + ret =devm_iio_trigger_register (indio_dev->dev.parent, st->trig); +if (ret) +return ret; + + indio_dev->trig =iio_trigger_get (st->trig); + +return devm_iio_triggered_buffer_setup (indio_dev->dev.parent, indio_dev, + &iio_pollfunc_store_time, + &ad7768_trigger_handler, + &ad7768_buffer_ops); +} +static int ad7768_probe (struct spi_device *spi) {struct ad7768_state *st;
@@ -689,20 +714,6 @@ static int ad7768_probe(struct spi_device *spi)return ret; } - st->trig =devm_iio_trigger_alloc (&spi->dev,"%s-dev%d" , - indio_dev->name, -iio_device_id (indio_dev)); -if (!st->trig) -return -ENOMEM ; - - st->trig->ops = &ad7768_trigger_ops; -iio_trigger_set_drvdata (st->trig, indio_dev); - ret =devm_iio_trigger_register (&spi->dev, st->trig); -if (ret) -return ret; - - indio_dev->trig =iio_trigger_get (st->trig); -init_completion (&st->completion); ret =ad7768_set_channel_label (indio_dev,ARRAY_SIZE (ad7768_channels));
@@ -716,10 +727,7 @@ static int ad7768_probe(struct spi_device *spi)if (ret)return ret; - ret =devm_iio_triggered_buffer_setup (&spi->dev, indio_dev, - &iio_pollfunc_store_time, - &ad7768_trigger_handler, - &ad7768_buffer_ops); + ret =ad7768_triggered_buffer_alloc (indio_dev);if (ret)return ret;
Signature · 1 line
2.34.1
Re: [PATCH v5 07/14] iio: adc: ad7768-1: Move buffer allocation to a separate function
Quoted 10 lines · On Fri, 11 Apr 2025 12:57:23 -0300 Jonathan Santos…
From: Sergiu Cuciurean <[email protected]>
This change moves the buffer allocation and related trigger allocation in a separate function, making space for adding another type of iio buffer if needed.
Reviewed-by Marcelo Schmitt <[email protected]>Reviewed-by David Lechner <[email protected]>Signed-off-by Sergiu Cuciurean <[email protected]>Signed-off-by Jonathan Santos <[email protected]>
Applied.
Given the next patch uses the new dt binding (which is queued behind bindings we are still discussing) I'll stop here.
Hopefully having merged a few patches will make it easier to focus in on the stuff that needs more review!
It also makes patchwork look less scary :)
Jonathan
PATCH v5 8/14 iio: adc: ad7768-1: add regulator to control VCM output
In reply to Jonathan SantosThe VCM output voltage can be used as a common-mode voltage within the amplifier preconditioning circuits external to the AD7768-1.
This change allows the user to configure VCM output using the regulator framework.
---
v5 Changes:
* enforce AD7768_REG_ANALOG2_VCM macro argument evaluation.
* switched to the new iio_device_claim/release_direct() functions.
v4 Changes:
* Added iio_device_claim_direct_mode() to regulator callbacks to avoid register access while in buffered mode.
* Changed regulator name to "ad7768-1-vcm".
* When regulator enable is called, it will set the last voltage selector configured.
* Disabled regulator before configuring it.
* Adressed other nits.
v3 Changes:
* Register VCM output via the regulator framework for improved flexibility and external integration.
v2 Changes:
* VCM output support is now defined by a devicetree property, instead of and IIO attribute.
--- drivers/iio/adc/Kconfig | 1 + drivers/iio/adc/ad7768-1.c | 176 +++++++++++++++++++++++++++++++++++++ 2 files changed, 177 insertions(+)
drivers/iio/adc/Kconfig +1 -0
@@ -322,6 +322,7 @@ config AD7766 config AD7768_1 tristate "Analog Devices AD7768-1 ADC driver" depends on SPI + select REGULATOR select REGMAP_SPI select IIO_BUFFER select IIO_TRIGGER
drivers/iio/adc/ad7768-1.c +176 -0
208 lines not on this page · raw message
Signature · 1 line
2.34.1
David Lechner
maintainerRe: [PATCH v5 08/14] iio: adc: ad7768-1: add regulator to control VCM output
Quoted 9 lines · Jonathan Santos
The VCM output voltage can be used as a common-mode voltage within the amplifier preconditioning circuits external to the AD7768-1.
This change allows the user to configure VCM output using the regulator framework.
Acked-by Marcelo Schmitt <[email protected]>Signed-off-by Jonathan Santos <[email protected]>---
...
drivers/iio/adc/ad7768-1.c static int ad7768_triggered_buffer_alloc(struct iio_dev *indio_dev) · line 709
Quoted 13 lines
drivers/iio/adc/ad7768-1.c +13
-0+static int ad7768_vcm_is_enabled (struct regulator_dev *rdev) +{ +struct iio_dev *indio_dev =rdev_get_drvdata (rdev); +struct ad7768_state *st =iio_priv (indio_dev); +int ret, val; + +if (!indio_dev) +return -EINVAL ; + +if (!iio_device_claim_direct (indio_dev)) +return -EBUSY ; + + ret =regmap_read (st->regmap,AD7768_REG_ANALOG2 , &val);
If we put iio_device_release_direct(indio_dev); here, we can avoid the goto.
Quoted 18 lines
drivers/iio/adc/ad7768-1.c +18
-0+if (ret) + goto err_release; + + ret =FIELD_GET (AD7768_REG_ANALOG2_VCM_MSK , val) !=AD7768_VCM_OFF ; +err_release: +iio_device_release_direct (indio_dev); + +return ret; +} + +static int ad7768_set_voltage_sel (struct regulator_dev *rdev, +unsigned int selector) +{ +unsigned int regval =AD7768_REG_ANALOG2_VCM (selector +1 ); +struct iio_dev *indio_dev =rdev_get_drvdata (rdev); +struct ad7768_state *st =iio_priv (indio_dev); +int ret; +
drivers/iio/adc/ad7768-1.c static int ad7768_triggered_buffer_alloc(struct iio_dev *indio_dev) · line 737
Quoted 10 lines
drivers/iio/adc/ad7768-1.c +10
-0+if (!indio_dev) +return -EINVAL ; + +if (!iio_device_claim_direct (indio_dev)) +return -EBUSY ; + + ret =regmap_update_bits (st->regmap,AD7768_REG_ANALOG2 , +AD7768_REG_ANALOG2_VCM_MSK , regval); +iio_device_release_direct (indio_dev); + st->vcm_output_sel = selector;
Do we still want to make this assignment if there is an error?
Quoted 9 lines
drivers/iio/adc/ad7768-1.c +9
-0+ +return ret; +} + +static int ad7768_get_voltage_sel (struct regulator_dev *rdev) +{ +struct iio_dev *indio_dev =rdev_get_drvdata (rdev); +struct ad7768_state *st =iio_priv (indio_dev); +int ret, val;
drivers/iio/adc/ad7768-1.c static int ad7768_triggered_buffer_alloc(struct iio_dev *indio_dev) · line 756
Quoted 10 lines
drivers/iio/adc/ad7768-1.c +10
-0+ +if (!indio_dev) +return -EINVAL ; + +if (!iio_device_claim_direct (indio_dev)) +return -EBUSY ; + + ret =regmap_read (st->regmap,AD7768_REG_ANALOG2 , &val); +if (ret) + goto err_release;
Can rearrange to avoid goto here too.
Quoted 9 lines
drivers/iio/adc/ad7768-1.c +9
-0+ + val =FIELD_GET (AD7768_REG_ANALOG2_VCM_MSK , val); + ret =clamp (val,1 , (int )rdev->desc->n_voltages) -1 ; +err_release: +iio_device_release_direct (indio_dev); + +return ret; +} +
...
Quoted 4 lines
drivers/iio/adc/ad7768-1.c +4
-0+static int ad7768_register_regulators (struct device *dev,struct ad7768_state *st, +struct iio_dev *indio_dev) +{ +struct regulator_config config = {
drivers/iio/adc/ad7768-1.c static int ad7768_triggered_buffer_alloc(struct iio_dev *indio_dev) · line 808
Quoted 10 lines
drivers/iio/adc/ad7768-1.c +10
-0+ .dev = dev, + .driver_data = indio_dev, + }; +int ret; + +/* Disable the regulator before registering it */ + ret =regmap_update_bits (st->regmap,AD7768_REG_ANALOG2 , +AD7768_REG_ANALOG2_VCM_MSK ,AD7768_VCM_OFF ); +if (ret) +return -EINVAL ;
Why not return ret; ?
Quoted 9 lines
drivers/iio/adc/ad7768-1.c +9
-0+ + st->vcm_rdev =devm_regulator_register (dev, &vcm_desc, &config); +if (IS_ERR (st->vcm_rdev)) +return dev_err_probe (dev,PTR_ERR (st->vcm_rdev), +"failed to register VCM regulator\n" ); + +return 0 ; +} +
Jonathan Cameron
maintainerRe: [PATCH v5 08/14] iio: adc: ad7768-1: add regulator to control VCM output
In reply to Jonathan SantosQuoted 8 lines · On Fri, 11 Apr 2025 12:57:42 -0300 Jonathan Santos…
The VCM output voltage can be used as a common-mode voltage within the amplifier preconditioning circuits external to the AD7768-1.
This change allows the user to configure VCM output using the regulator framework.
Acked-by Marcelo Schmitt <[email protected]>Signed-off-by Jonathan Santos <[email protected]>
Just one question from me inline.
drivers/iio/adc/ad7768-1.c static int ad7768_triggered_buffer_alloc(struct iio_dev *indio_dev) · line 787
Quoted 9 lines
drivers/iio/adc/ad7768-1.c +4
-0+static const struct regulator_desc vcm_desc = { + .name ="ad7768-1-vcm" , + .of_match =of_match_ptr ("vcm-output" ), + .regulators_node =of_match_ptr ("regulators" ),
I see this use of of_match_ptr() is common but not universal in regulator drivers. Any idea if it really is just to save a string or if there some more fundamental reason to make these go away when device tree support is not built?
We spent a while getting other uses of this out of IIO so I'd like to know more about this one!
Jonathan
Quoted 6 lines
drivers/iio/adc/ad7768-1.c +6
-0+ .n_voltages = ARRAY_SIZE(vcm_voltage_table), + .volt_table = vcm_voltage_table, + .ops = &vcm_regulator_ops, + .type = REGULATOR_VOLTAGE, + .owner = THIS_MODULE, +};
PATCH v5 9/14 iio: adc: ad7768-1: Add GPIO controller support
In reply to Jonathan SantosFrom: Sergiu Cuciurean <[email protected]>
The AD7768-1 has the ability to control other local hardware (such as gain stages),to power down other blocks in the signal chain, or read local status signals over the SPI interface.
Add direct mode conditional locks in the gpio callbacks to prevent register access when the device is in buffered mode.
This change exports the AD7768-1's four gpios and makes them accessible at an upper layer.
---
v5 Changes:
* Use the new new iio_device_claim/release_direct() functions.
* replaced gpiochip_add_data() for devm_gpiochip_add_data().
v4 Changes:
* Mentioned in the commit message that we cannot tweak the GPIO controller when the device is not in direct mode.
v3 Changes:
* Fixed SoB order.
* Added mising iio_device_release_direct_mode().
* Simplified some regmap writes.
* Removed ad7768_gpio_request() callback.
* Fixed line wrapping.
v2 Changes:
* Replaced mutex for iio_device_claim_direct_mode().
* Use gpio-controller property to conditionally enable the GPIO support.
* OBS: when the GPIO is configured as output, we should read the current state value from AD7768_REG_GPIO_WRITE.
--- drivers/iio/adc/ad7768-1.c | 139 ++++++++++++++++++++++++++++++++++++- 1 file changed, 137 insertions(+), 2 deletions(-)
drivers/iio/adc/ad7768-1.c +137 -2
@@ -9,6 +9,8 @@#include <linux/delay.h> #include <linux/device.h> #include <linux/err.h> +#include <linux/gpio.h> +#include <linux/gpio/driver.h> #include <linux/gpio/consumer.h> #include <linux/kernel.h> #include <linux/module.h>
@@ -86,6 +88,16 @@#define AD7768_REG_ANALOG2_VCM_MSK GENMASK(2, 0)#define AD7768_REG_ANALOG2_VCM (x) FIELD_PREP(AD7768_REG_ANALOG2_VCM_MSK, (x)) +/* AD7768_REG_GPIO_CONTROL */ +#define AD7768_GPIO_UNIVERSAL_EN BIT(7) +#define AD7768_GPIO_CONTROL_MSK GENMASK(3, 0) + +/* AD7768_REG_GPIO_WRITE */ +#define AD7768_GPIO_WRITE_MSK GENMASK(3, 0) + +/* AD7768_REG_GPIO_READ */ +#define AD7768_GPIO_READ_MSK GENMASK(3, 0) +#define AD7768_VCM_OFF 0x07enum ad7768_conv_mode {
@@ -168,6 +180,7 @@ struct ad7768_state {struct regulator_dev *vcm_rdev;unsigned int vcm_output_sel;struct clk *mclk; +struct gpio_chip gpiochip;unsigned int mclk_freq;unsigned int samp_freq;struct completion completion;
@@ -353,6 +366,120 @@ static int ad7768_set_dig_fil(struct ad7768_state *st,return 0 ; } +static int ad7768_gpio_direction_input (struct gpio_chip *chip,unsigned int offset) +{ +struct iio_dev *indio_dev =gpiochip_get_data (chip); +struct ad7768_state *st =iio_priv (indio_dev); +int ret; + +if (!iio_device_claim_direct (indio_dev)) +return -EBUSY ; + + ret =regmap_clear_bits (st->regmap,AD7768_REG_GPIO_CONTROL , +BIT (offset)); +iio_device_release_direct (indio_dev); + +return ret; +} + +static int ad7768_gpio_direction_output (struct gpio_chip *chip, +unsigned int offset,int value) +{ +struct iio_dev *indio_dev =gpiochip_get_data (chip); +struct ad7768_state *st =iio_priv (indio_dev); +int ret; + +if (!iio_device_claim_direct (indio_dev)) +return -EBUSY ; + + ret =regmap_set_bits (st->regmap,AD7768_REG_GPIO_CONTROL , +BIT (offset)); +iio_device_release_direct (indio_dev); + +return ret; +} + +static int ad7768_gpio_get (struct gpio_chip *chip,unsigned int offset) +{ +struct iio_dev *indio_dev =gpiochip_get_data (chip); +struct ad7768_state *st =iio_priv (indio_dev); +unsigned int val; +int ret; + +if (!iio_device_claim_direct (indio_dev)) +return -EBUSY ; + + ret =regmap_read (st->regmap,AD7768_REG_GPIO_CONTROL , &val); +if (ret) + goto err_release; + +/* +* If the GPIO is configured as an output, read the current value from +* AD7768_REG_GPIO_WRITE. Otherwise, read the input value from +* AD7768_REG_GPIO_READ. +*/ +if (val &BIT (offset)) + ret =regmap_read (st->regmap,AD7768_REG_GPIO_WRITE , &val); +else + ret =regmap_read (st->regmap,AD7768_REG_GPIO_READ , &val); +if (ret) + goto err_release; + + ret = !!(val &BIT (offset)); +err_release: +iio_device_release_direct (indio_dev); + +return ret; +} + +static void ad7768_gpio_set (struct gpio_chip *chip,unsigned int offset,int value) +{ +struct iio_dev *indio_dev =gpiochip_get_data (chip); +struct ad7768_state *st =iio_priv (indio_dev); +unsigned int val; +int ret; + +if (!iio_device_claim_direct (indio_dev)) +return ; + + ret =regmap_read (st->regmap,AD7768_REG_GPIO_CONTROL , &val); +if (ret) + goto err_release; + +if (val &BIT (offset)) +regmap_update_bits (st->regmap,AD7768_REG_GPIO_WRITE , +BIT (offset), value << offset); + +err_release: +iio_device_release_direct (indio_dev); +} + +static int ad7768_gpio_init (struct iio_dev *indio_dev) +{ +struct ad7768_state *st =iio_priv (indio_dev); +int ret; + + ret =regmap_write (st->regmap,AD7768_REG_GPIO_CONTROL , +AD7768_GPIO_UNIVERSAL_EN ); +if (ret) +return ret; + + st->gpiochip = (struct gpio_chip ) { + .label ="ad7768_1_gpios" , + .base =-1 , + .ngpio =4 , + .parent = &st->spi->dev, + .can_sleep = true, + .direction_input = ad7768_gpio_direction_input, + .direction_output = ad7768_gpio_direction_output, + .get = ad7768_gpio_get, + .set = ad7768_gpio_set, + .owner =THIS_MODULE , + }; + +return devm_gpiochip_add_data (&st->spi->dev, &st->gpiochip, indio_dev); +} +static int ad7768_set_freq (struct ad7768_state *st,unsigned int freq) {
@@ -494,8 +621,9 @@ static const struct iio_info ad7768_info = { .debugfs_reg_access = &ad7768_reg_access, }; -static int ad7768_setup (struct ad7768_state *st) +static int ad7768_setup (struct iio_dev *indio_dev) { +struct ad7768_state *st =iio_priv (indio_dev);int ret; st->gpio_reset = devm_gpiod_get_optional(&st->spi->dev, "reset",
@@ -528,6 +656,13 @@ static int ad7768_setup(struct ad7768_state *st)if (IS_ERR (st->gpio_sync_in))return PTR_ERR (st->gpio_sync_in); +/* Only create a Chip GPIO if flagged for it */ +if (device_property_read_bool (&st->spi->dev,"gpio-controller" )) { + ret =ad7768_gpio_init (indio_dev); +if (ret <0 ) +return ret; + } +/* Set the default sampling frequency to 32000 kSPS */ return ad7768_set_freq (st,32000 ); }
@@ -884,7 +1019,7 @@ static int ad7768_probe(struct spi_device *spi)if (ret)return ret; - ret =ad7768_setup (st); + ret =ad7768_setup (indio_dev);if (ret <0 ) {dev_err (&spi->dev,"AD7768 setup failed\n" );return ret;
Signature · 1 line
2.34.1
Re: [PATCH v5 09/14] iio: adc: ad7768-1: Add GPIO controller support
Hi Jonathan,
thanks for your patch!
Quoted 2 lines · Jonathan Santos
(quoted diff) +2
-0+static void ad7768_gpio_set(struct gpio_chip *chip, unsigned int offset, int value) +{
(...)
drivers/iio/adc/ad7768-1.c static int ad7768_set_dig_fil(struct ad7768_state *st, · line 476
Quoted 10 lines
drivers/iio/adc/ad7768-1.c +10
-0+ st->gpiochip = (struct gpio_chip ) { + .label ="ad7768_1_gpios" , + .base =-1 , + .ngpio =4 , + .parent = &st->spi->dev, + .can_sleep = true, + .direction_input = ad7768_gpio_direction_input, + .direction_output = ad7768_gpio_direction_output, + .get = ad7768_gpio_get, + .set = ad7768_gpio_set,
Due to refactorings going on in the .set calls please switch this to use the new .set_rv() callback that return an integer (errorcode) on failure.
Yours,
Linus Walleij
PATCH v5 10/14 iio: adc: ad7768-1: add multiple scan types to support 16-bits mode
In reply to Jonathan SantosWhen the device is configured to decimation x8, only possible in the sinc5 filter, output data is reduced to 16-bits in order to support 1 MHz of sampling frequency due to clock limitation.
Use multiple scan types feature to enable the driver to switch scan type in runtime, making possible to support both 24-bit and 16-bit resolution.
---
v5 Changes:
* None.
v4 Changes:
* None.
v3 Changes:
* Decreased storagebits to 16 for AD7768_SCAN_TYPE_HIGH_SPEED scan type.
v2 Changes:
* Included the ".shift" value back to scan_type.
* Changed the number of bytes from regmap_read instead of shifting the ADC sample value when the word size is lower (16-bits).
--- drivers/iio/adc/ad7768-1.c | 74 ++++++++++++++++++++++++++++++++------ 1 file changed, 64 insertions(+), 10 deletions(-)
drivers/iio/adc/ad7768-1.c +64 -10
@@ -139,6 +139,15 @@ struct ad7768_clk_configuration {enum ad7768_pwrmode pwrmode; }; +enum ad7768_scan_type { +AD7768_SCAN_TYPE_NORMAL , +AD7768_SCAN_TYPE_HIGH_SPEED , +}; + +static const int ad7768_mclk_div_rates[4 ] = { +16 ,8 ,4 ,2 , +}; +static const struct ad7768_clk_configuration ad7768_clk_config[] = { {AD7768_MCLK_DIV_2 ,AD7768_DEC_RATE_8 ,16 ,AD7768_FAST_MODE }, {AD7768_MCLK_DIV_2 ,AD7768_DEC_RATE_16 ,32 ,AD7768_FAST_MODE },
@@ -153,6 +162,22 @@ static const struct ad7768_clk_configuration ad7768_clk_config[] = { { AD7768_MCLK_DIV_16, AD7768_DEC_RATE_1024, 16384, AD7768_ECO_MODE }, }; +static const struct iio_scan_type ad7768_scan_type[] = { + [AD7768_SCAN_TYPE_NORMAL] = { + .sign = 's', + .realbits = 24, + .storagebits = 32, + .shift = 8, + .endianness = IIO_BE, + }, + [AD7768_SCAN_TYPE_HIGH_SPEED] = { + .sign = 's', + .realbits = 16, + .storagebits = 16, + .endianness = IIO_BE, + }, +}; + static const struct iio_chan_spec ad7768_channels[] = { { .type = IIO_VOLTAGE,
@@ -162,13 +187,9 @@ static const struct iio_chan_spec ad7768_channels[] = { .indexed = 1, .channel = 0, .scan_index = 0, - .scan_type = { - .sign = 's', - .realbits = 24, - .storagebits = 32, - .shift = 8, - .endianness = IIO_BE, - }, + .has_ext_scan_type = 1, + .ext_scan_type = ad7768_scan_type, + .num_ext_scan_type = ARRAY_SIZE(ad7768_scan_type), }, };
@@ -182,6 +203,7 @@ struct ad7768_state {struct clk *mclk;struct gpio_chip gpiochip;unsigned int mclk_freq; +unsigned int dec_rate;unsigned int samp_freq;struct completion completion;struct iio_trigger *trig;
@@ -300,6 +322,15 @@ static int ad7768_scan_direct(struct iio_dev *indio_dev)if (ret)return ret; +/* +* When the decimation rate is set to x8, the ADC data precision is +* reduced from 24 bits to 16 bits. Since the AD7768_REG_ADC_DATA +* register provides 24-bit data, the precision is reduced by +* right-shifting the read value by 8 bits. +*/ +if (st->dec_rate ==8 ) + readval >>=8 ; +/* * Any SPI configuration of the AD7768-1 can only be * performed in continuous conversion mode.
@@ -514,6 +545,8 @@ static int ad7768_set_freq(struct ad7768_state *st,if (ret <0 )return ret; + st->dec_rate = ad7768_clk_config[idx].clk_div / + ad7768_mclk_div_rates[ad7768_clk_config[idx].mclk_div]; st->samp_freq =DIV_ROUND_CLOSEST (st->mclk_freq, ad7768_clk_config[idx].clk_div);
@@ -547,8 +580,13 @@ static int ad7768_read_raw(struct iio_dev *indio_dev, int *val, int *val2, long info) { struct ad7768_state *st = iio_priv(indio_dev); +const struct iio_scan_type *scan_type;int scale_uv, ret; + scan_type =iio_get_current_scan_type (indio_dev, chan); +if (IS_ERR (scan_type)) +return PTR_ERR (scan_type); +switch (info) {case IIO_CHAN_INFO_RAW :if (!iio_device_claim_direct (indio_dev))
@@ -559,7 +597,7 @@ static int ad7768_read_raw(struct iio_dev *indio_dev,iio_device_release_direct (indio_dev);if (ret <0 )return ret; - *val =sign_extend32 (ret, chan->scan_type.realbits -1 ); + *val =sign_extend32 (ret, scan_type->realbits -1 );return IIO_VAL_INT ;
@@ -569,7 +607,7 @@ static int ad7768_read_raw(struct iio_dev *indio_dev,return scale_uv; *val = (scale_uv *2 ) /1000 ; - *val2 = chan->scan_type.realbits; + *val2 = scan_type->realbits;return IIO_VAL_FRACTIONAL_LOG2 ;
@@ -613,11 +651,21 @@ static const struct attribute_group ad7768_group = { .attrs = ad7768_attributes, }; +static int ad7768_get_current_scan_type (const struct iio_dev *indio_dev, +const struct iio_chan_spec *chan) +{ +struct ad7768_state *st =iio_priv (indio_dev); + +return st->dec_rate ==8 ?AD7768_SCAN_TYPE_HIGH_SPEED : +AD7768_SCAN_TYPE_NORMAL ; +} +static const struct iio_info ad7768_info = { .attrs = &ad7768_group, .read_raw = &ad7768_read_raw, .write_raw = &ad7768_write_raw, .read_label = ad7768_read_label, + .get_current_scan_type = &ad7768_get_current_scan_type, .debugfs_reg_access = &ad7768_reg_access, };
@@ -672,9 +720,15 @@ static irqreturn_t ad7768_trigger_handler(int irq, void *p)struct iio_poll_func *pf = p;struct iio_dev *indio_dev = pf->indio_dev;struct ad7768_state *st =iio_priv (indio_dev); +const struct iio_scan_type *scan_type;int ret; - ret =spi_read (st->spi, &st->data.scan.chan,3 ); + scan_type =iio_get_current_scan_type (indio_dev, &indio_dev->channels[0 ]); +if (IS_ERR (scan_type)) +return PTR_ERR (scan_type); + + ret =spi_read (st->spi, &st->data.scan.chan, +BITS_TO_BYTES (scan_type->realbits));if (ret <0 ) goto out;
Signature · 1 line
2.34.1
PATCH v5 11/14 iio: adc: ad7768-1: add support for Synchronization over SPI
In reply to Jonathan SantosThe synchronization method using GPIO requires the generated pulse to be truly synchronous with the base MCLK signal. When it is not possible to do that in hardware, the datasheet recommends using synchronization over SPI, where the generated pulse is already synchronous with MCLK. This requires the SYNC_OUT pin to be connected to SYNC_IN pin.
Use trigger-sources property to enable device synchronization over SPI and to replace sync-in-gpios property for a gpio-trigger node.
---
v5 Changes:
* Allow omitting trigger-sources property.
* include gpio-trigger to trigger-sources to replace adi,sync-in-gpios property.
* Read trigger-sources cell value to differentiate the trigger type.
v4 Changes:
* None.
v3 Changes:
* Fixed args.fwnode leakage in the trigger-sources parsing.
* Synchronization over spi is enabled when the trigger-sources references the own device.
* Synchronization is kept within the device, and return error if the gpio is not defined and the trigger-sources reference does not match the current device.
v2 Changes:
* Synchronization via SPI is enabled when the Sync GPIO is not defined.
* now trigger-sources property indicates the synchronization provider or main device. The main device will be used to drive the SYNC_IN when requested (via GPIO or SPI).
--- drivers/iio/adc/ad7768-1.c | 100 ++++++++++++++++++++++++++++++++++--- 1 file changed, 92 insertions(+), 8 deletions(-)
drivers/iio/adc/ad7768-1.c +92 -8
@@ -100,6 +100,11 @@#define AD7768_VCM_OFF 0x07 +enum ad7768_trigger_type { +AD7768_TRIGGER_TYPE_GPIO , +AD7768_TRIGGER_TYPE_DEV +}; +enum ad7768_conv_mode {AD7768_CONTINUOUS ,AD7768_ONE_SHOT ,
@@ -209,6 +214,7 @@ struct ad7768_state {struct iio_trigger *trig;struct gpio_desc *gpio_sync_in;struct gpio_desc *gpio_reset; +bool en_spi_sync;const char *labels[ARRAY_SIZE (ad7768_channels)];/* * DMA (thus cache coherency maintenance) may require the
@@ -295,6 +301,19 @@ static const struct regmap_config ad7768_regmap24_config = { .max_register = AD7768_REG24_COEFF_DATA, }; +static int ad7768_send_sync_pulse (struct ad7768_state *st) +{ +if (st->en_spi_sync) +return regmap_write (st->regmap,AD7768_REG_SYNC_RESET ,0x00 ); + +if (st->gpio_sync_in) { +gpiod_set_value_cansleep (st->gpio_sync_in,1 ); +gpiod_set_value_cansleep (st->gpio_sync_in,0 ); + } + +return 0 ; +} +static int ad7768_set_mode (struct ad7768_state *st,enum ad7768_conv_mode mode) {
@@ -391,10 +410,7 @@ static int ad7768_set_dig_fil(struct ad7768_state *st,return ret;/* A sync-in pulse is required every time the filter dec rate changes */ -gpiod_set_value (st->gpio_sync_in,1 ); -gpiod_set_value (st->gpio_sync_in,0 ); - -return 0 ; +return ad7768_send_sync_pulse (st); }static int ad7768_gpio_direction_input (struct gpio_chip *chip,unsigned int offset)
@@ -669,6 +685,75 @@ static const struct iio_info ad7768_info = { .debugfs_reg_access = &ad7768_reg_access, }; +static int ad7768_set_sync_source (struct device *dev,struct ad7768_state *st) +{ +struct fwnode_reference_args args; +struct fwnode_handle *fwnode =NULL ; +const char *value; +int trigger_type, ret; + +/* +* The AD7768-1 allows two primary methods for driving the SYNC_IN pin +* to synchronize one or more devices: +* 1. Using a GPIO to directly drive the SYNC_IN pin. +* 2. Using a SPI command, where the SYNC_OUT pin generates a +* synchronization pulse that loops back to the SYNC_IN pin. +*/ + ret =fwnode_property_get_reference_args (dev_fwnode (dev), +"trigger-sources" , +"#trigger-source-cells" , +0 ,0 , &args); +if (ret) { +/* +* In the absence of trigger-sources property, enable self +* synchronziation via SPI command. +*/ + st->en_spi_sync = true; +return 0 ; + } + + trigger_type = args.args[0 ]; + fwnode = args.fwnode; +if (!fwnode) +return dev_err_probe (dev, -ENOENT , +"Invalid or missing fwnode in 'trigger-sources'\n" ); + +switch (trigger_type) { +case AD7768_TRIGGER_TYPE_GPIO : + ret =fwnode_property_read_string (fwnode,"compatible" , &value); +if (ret) + goto out_put_node; + +if (strcmp ("gpio-trigger" , value)) + goto err_not_supp; + + st->gpio_sync_in =fwnode_gpiod_get_index (fwnode,NULL ,0 , +GPIOD_OUT_LOW , +"sync-in" ); +if (IS_ERR (st->gpio_sync_in)) + ret =PTR_ERR (st->gpio_sync_in); + + goto out_put_node; +case AD7768_TRIGGER_TYPE_DEV : +/* Only self synchronization is supported for now */ +if (fwnode->dev == dev) { + st->en_spi_sync = true; + goto out_put_node; + } + + goto err_not_supp; +default : + goto err_not_supp; + } + +err_not_supp: + ret = -EOPNOTSUPP ; +dev_err (dev,"Trigger-sources type not supported\n" ); +out_put_node: +fwnode_handle_put (args.fwnode); +return ret; +} +static int ad7768_setup (struct iio_dev *indio_dev) {struct ad7768_state *st =iio_priv (indio_dev);
@@ -699,10 +784,9 @@ static int ad7768_setup(struct iio_dev *indio_dev)return ret; } - st->gpio_sync_in =devm_gpiod_get (&st->spi->dev,"adi,sync-in" , -GPIOD_OUT_LOW ); -if (IS_ERR (st->gpio_sync_in)) -return PTR_ERR (st->gpio_sync_in); + ret =ad7768_set_sync_source (&st->spi->dev, st); +if (ret) +return ret;/* Only create a Chip GPIO if flagged for it */ if (device_property_read_bool (&st->spi->dev,"gpio-controller" )) {
Signature · 1 line
2.34.1
David Lechner
maintainerRe: [PATCH v5 11/14] iio: adc: ad7768-1: add support for Synchronization over SPI
Quoted 6 lines · Jonathan Santos
The synchronization method using GPIO requires the generated pulse to be truly synchronous with the base MCLK signal. When it is not possible to do that in hardware, the datasheet recommends using synchronization over SPI, where the generated pulse is already synchronous with MCLK. This requires the
SYNC_OUTpin to be connected toSYNC_INpin.
Quoted 7 lines
Use trigger-sources property to enable device synchronization over SPI and to replace sync-in-gpios property for a gpio-trigger node.
Signed-off-by Jonathan Santos <[email protected]>---
v5 Changes:
* Allow omitting trigger-sources property.
I didn't see this possibility mentioned in the dt-bindings patch. Maybe I missed something?
Quoted 4 lines
* include gpio-trigger to trigger-sources to replace adi,sync-in-gpios property.
* Read trigger-sources cell value to differentiate the trigger type.
...
Quoted 8 lines
drivers/iio/adc/ad7768-1.c | 100 ++++++++++++++++++++++++++++++++++--- 1 file changed, 92 insertions(+), 8 deletions(-)drivers/iio/adc/ad7768-1.c +0
-0
drivers/iio/adc/ad7768-1.c line 106
Quoted 7 lines
drivers/iio/adc/ad7768-1.c +4
-0@@ -100,6 +100,11 @@#define AD7768_VCM_OFF 0x07 +enum ad7768_trigger_type { +AD7768_TRIGGER_TYPE_GPIO , +AD7768_TRIGGER_TYPE_DEV +};
I assume this is supposed to correspond to the #trigger-source-cells argument? It would make sense to put this in a dt-bindings/iio/adc/adi,ad7768-1.h file in the dt-bindings patch so that it can also be used in .dts files.
Quoted 8 lines
drivers/iio/adc/ad7768-1.c +8
-0+static int ad7768_set_sync_source (struct device *dev,struct ad7768_state *st) +{ +struct fwnode_reference_args args; +struct fwnode_handle *fwnode =NULL ; +const char *value; +int trigger_type, ret; + +/*
drivers/iio/adc/ad7768-1.c static const struct iio_info ad7768_info = { · line 705
Quoted 10 lines
drivers/iio/adc/ad7768-1.c +10
-0+* The AD7768-1 allows two primary methods for driving the SYNC_IN pin +* to synchronize one or more devices: +* 1. Using a GPIO to directly drive the SYNC_IN pin. +* 2. Using a SPI command, where the SYNC_OUT pin generates a +* synchronization pulse that loops back to the SYNC_IN pin. +*/ + ret =fwnode_property_get_reference_args (dev_fwnode (dev), +"trigger-sources" , +"#trigger-source-cells" , +0 ,0 , &args);
Since we have more than one possible trigger-source index, it could be helpful to create macros or an enum instead of hard-coding 0 here for the index arg.
drivers/iio/adc/ad7768-1.c static const struct iio_info ad7768_info = { · line 706
Quoted 1 line
drivers/iio/adc/ad7768-1.c +1
-0+if (ret) {
This ignores errors other than the property just being missing. Probably better would be to use device_property_present() first for this check then call fwnode_property_get_reference_args() later and return any error.
drivers/iio/adc/ad7768-1.c static const struct iio_info ad7768_info = { · line 709
Quoted 3 lines
drivers/iio/adc/ad7768-1.c +3
-0+/* +* In the absence of trigger-sources property, enable self +* synchronziation via SPI command.
s/synchronziation/synchronization/
drivers/iio/adc/ad7768-1.c static const struct iio_info ad7768_info = { · line 715
Quoted 6 lines
drivers/iio/adc/ad7768-1.c +6
-0+*/ + st->en_spi_sync = true; +return 0 ; + } + + trigger_type = args.args[0 ];
This is accessing args.args without checking args.nargs, so could be reading out of bounds.
Also, we need to inspect args.fwnode first to see what type of trigger it is since the meaning of the args will depend on the compatible string of args.fwnode. The "gpio-trigger" has #trigger-source-cells = <0>; so this really would be reading uninitialized memory.
drivers/iio/adc/ad7768-1.c static const struct iio_info ad7768_info = { · line 719
Quoted 4 lines
drivers/iio/adc/ad7768-1.c +4
-0+ fwnode = args.fwnode; +if (!fwnode) +return dev_err_probe (dev, -ENOENT , +"Invalid or missing fwnode in 'trigger-sources'\n" );
Is it really possible for this to happen without an error being returned by fwnode_property_get_reference_args()?
drivers/iio/adc/ad7768-1.c static const struct iio_info ad7768_info = { · line 722
Quoted 3 lines
drivers/iio/adc/ad7768-1.c +3
-0+ +switch (trigger_type) { +case AD7768_TRIGGER_TYPE_GPIO :
As mentioned in the dt-bindings review, this isn't a valid value. We should just check the compatible string directly.
drivers/iio/adc/ad7768-1.c static const struct iio_info ad7768_info = { · line 728
Quoted 6 lines
drivers/iio/adc/ad7768-1.c +6
-0+ ret =fwnode_property_read_string (fwnode,"compatible" , &value); +if (ret) + goto out_put_node; + +if (strcmp ("gpio-trigger" , value)) + goto err_not_supp;
Ideally we would have a trigger subsystem where we could just call a devm_trigger_source_get_gpio() function instead of this. But that is a bit beyond the scope of this patch series. :-)
drivers/iio/adc/ad7768-1.c
Quoted 2 lines
drivers/iio/adc/ad7768-1.c +2
-0+ + st->gpio_sync_in = fwnode_gpiod_get_index(fwnode, NULL, 0,
Should be devm_fwnode_gpiod_get_index() so that the gpiod is released on driver removal.
drivers/iio/adc/ad7768-1.c static const struct iio_info ad7768_info = { · line 737
Quoted 7 lines
drivers/iio/adc/ad7768-1.c +7
-0+GPIOD_OUT_LOW , + "sync-in"); +if (IS_ERR (st->gpio_sync_in)) + ret =PTR_ERR (st->gpio_sync_in); + + goto out_put_node; +case AD7768_TRIGGER_TYPE_DEV :
As mentioned in the dt-bindings review, this isn't a valid value. We can just check fwnode->dev directly earlier in this function. Then it is only safe to do trigger_type = args.args[0]; after that. And we should be checking that trigger_type is SYNC_OUT since that is the only supported self trigger for now.
Quoted 23 lines
drivers/iio/adc/ad7768-1.c +19
-0+/* Only self synchronization is supported for now */ +if (fwnode->dev == dev) { + st->en_spi_sync = true; + goto out_put_node; + } + + goto err_not_supp; +default : + goto err_not_supp; + } + +err_not_supp: + ret = -EOPNOTSUPP ; +dev_err (dev,"Trigger-sources type not supported\n" ); +out_put_node: +fwnode_handle_put (args.fwnode); +return ret; +} +static int ad7768_setup (struct iio_dev *indio_dev) {struct ad7768_state *st =iio_priv (indio_dev);
drivers/iio/adc/ad7768-1.c static int ad7768_setup(struct iio_dev *indio_dev) · line 702
Quoted 4 lines
drivers/iio/adc/ad7768-1.c +0
-1@@ -699,10 +784,9 @@ static int ad7768_setup(struct iio_dev *indio_dev)return ret; } - st->gpio_sync_in = devm_gpiod_get(&st->spi->dev, "adi,sync-in",
I don't see where this function is moved to. If we remove it, wouldn't that break existing users depending on it? We can't expect everyone to update their devicetree to the new bindings.
We could change it to devm_gpiod_get_optional() instead and only call ad7768_set_sync_source() if (!st->gpio_sync_in).
Quoted 9 lines
drivers/iio/adc/ad7768-1.c +3
-3- GPIOD_OUT_LOW); -if (IS_ERR (st->gpio_sync_in)) -return PTR_ERR (st->gpio_sync_in); + ret =ad7768_set_sync_source (&st->spi->dev, st); +if (ret) +return ret;/* Only create a Chip GPIO if flagged for it */ if (device_property_read_bool (&st->spi->dev,"gpio-controller" )) {
PATCH v5 12/14 iio: adc: ad7768-1: replace manual attribute declaration
In reply to Jonathan SantosUse read_avail callback from struct iio_info to replace the manual declaration of sampling_frequency_available attribute.
---
v5 Changes:
* none.
v4 Changes:
* Added ad7768_fill_samp_freq_tbl() helper function.
* Sampling frequency table is precomputed at probe.
v3 Changes:
* New patch in v3.
--- drivers/iio/adc/ad7768-1.c | 63 +++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 32 deletions(-)
drivers/iio/adc/ad7768-1.c +31 -32
@@ -189,6 +189,7 @@ static const struct iio_chan_spec ad7768_channels[] = { .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), + .info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ), .indexed = 1, .channel = 0, .scan_index = 0,
@@ -210,6 +211,7 @@ struct ad7768_state {unsigned int mclk_freq;unsigned int dec_rate;unsigned int samp_freq; +unsigned int samp_freq_avail[ARRAY_SIZE (ad7768_clk_config)];struct completion completion;struct iio_trigger *trig;struct gpio_desc *gpio_sync_in;
@@ -314,6 +316,15 @@ static int ad7768_send_sync_pulse(struct ad7768_state *st)return 0 ; } +static void ad7768_fill_samp_freq_tbl (struct ad7768_state *st) +{ +int i; + +for (i =0 ; i <ARRAY_SIZE (ad7768_clk_config); i++) + st->samp_freq_avail[i] =DIV_ROUND_CLOSEST (st->mclk_freq, + ad7768_clk_config[i].clk_div); +} +static int ad7768_set_mode (struct ad7768_state *st,enum ad7768_conv_mode mode) {
@@ -569,28 +580,6 @@ static int ad7768_set_freq(struct ad7768_state *st,return 0 ; } -static ssize_t ad7768_sampling_freq_avail (struct device *dev, -struct device_attribute *attr, -char *buf) -{ -struct iio_dev *indio_dev =dev_to_iio_dev (dev); -struct ad7768_state *st =iio_priv (indio_dev); -unsigned int freq; -int i, len =0 ; - -for (i =0 ; i <ARRAY_SIZE (ad7768_clk_config); i++) { - freq =DIV_ROUND_CLOSEST (st->mclk_freq, - ad7768_clk_config[i].clk_div); - len +=scnprintf (buf + len,PAGE_SIZE - len,"%d " , freq); - } - - buf[len -1 ] ='\n' ; - -return len; -} - -static IIO_DEV_ATTR_SAMP_FREQ_AVAIL (ad7768_sampling_freq_avail); -static int ad7768_read_raw (struct iio_dev *indio_dev,struct iio_chan_spec const *chan,int *val,int *val2,long info)
@@ -636,6 +625,24 @@ static int ad7768_read_raw(struct iio_dev *indio_dev,return -EINVAL ; } +static int ad7768_read_avail (struct iio_dev *indio_dev, +struct iio_chan_spec const *chan, +const int **vals,int *type,int *length, +long info) +{ +struct ad7768_state *st =iio_priv (indio_dev); + +switch (info) { +case IIO_CHAN_INFO_SAMP_FREQ : + *vals = (int *)st->samp_freq_avail; + *length =ARRAY_SIZE (ad7768_clk_config); + *type =IIO_VAL_INT ; +return IIO_AVAIL_LIST ; +default : +return -EINVAL ; + } +} +static int ad7768_write_raw (struct iio_dev *indio_dev,struct iio_chan_spec const *chan,int val,int val2,long info)
@@ -658,15 +665,6 @@ static int ad7768_read_label(struct iio_dev *indio_dev,return sprintf (label,"%s\n" , st->labels[chan->channel]); } -static struct attribute *ad7768_attributes[] = { - &iio_dev_attr_sampling_frequency_available.dev_attr.attr, -NULL -}; - -static const struct attribute_group ad7768_group = { - .attrs = ad7768_attributes, -}; -static int ad7768_get_current_scan_type (const struct iio_dev *indio_dev,const struct iio_chan_spec *chan) {
@@ -677,8 +675,8 @@ static int ad7768_get_current_scan_type(const struct iio_dev *indio_dev, }static const struct iio_info ad7768_info = { - .attrs = &ad7768_group, .read_raw = &ad7768_read_raw, + .read_avail = &ad7768_read_avail, .write_raw = &ad7768_write_raw, .read_label = ad7768_read_label, .get_current_scan_type = &ad7768_get_current_scan_type,
@@ -1145,6 +1143,7 @@ static int ad7768_probe(struct spi_device *spi)return PTR_ERR (st->mclk); st->mclk_freq =clk_get_rate (st->mclk); +ad7768_fill_samp_freq_tbl (st); indio_dev->channels = ad7768_channels; indio_dev->num_channels =ARRAY_SIZE (ad7768_channels);
Signature · 1 line
2.34.1
PATCH v5 13/14 iio: adc: ad7768-1: add filter type and oversampling ratio attributes
In reply to Jonathan SantosSeparate filter type and decimation rate from the sampling frequency attribute. The new filter type attribute enables sinc3, sinc3+rej60 and wideband filters, which were previously unavailable.
Previously, combining decimation and MCLK divider in the sampling frequency obscured performance trade-offs. Lower MCLK divider settings increase power usage, while lower decimation rates reduce precision by decreasing averaging. By creating an oversampling attribute, which controls the decimation, users gain finer control over performance.
The addition of those attributes allows a wider range of sampling frequencies and more access to the device features. Sampling frequency table is updated after every digital filter paramerter change.
---
v5 Changes:
* Adressed some nits.
* Use the new new iio_device_claim/release_direct() functions.
v4 Changes:
* Sampling frequency table is dynamically updated after every filter configuration.
v3 Changes:
* removed unsed variables.
* included sinc3+rej60 filter type.
* oversampling_ratio moved to info_mask_shared_by_type.
* reordered functions to avoid foward declaration.
* simplified regmap writes.
* Removed locking.
* replaced some helper functions for direct regmap_update_bits calls.
* Addressed other nits.
v2 Changes:
* Decimation_rate attribute replaced for oversampling_ratio.
--- drivers/iio/adc/ad7768-1.c | 359 +++++++++++++++++++++++++++++-------- 1 file changed, 289 insertions(+), 70 deletions(-)
drivers/iio/adc/ad7768-1.c +289 -70
@@ -20,6 +20,7 @@#include <linux/regulator/driver.h> #include <linux/sysfs.h> #include <linux/spi/spi.h> +#include <linux/util_macros.h> #include <linux/iio/buffer.h> #include <linux/iio/iio.h>
@@ -75,11 +76,15 @@#define AD7768_PWR_PWRMODE (x) FIELD_PREP(AD7768_PWR_PWRMODE_MSK, x)/* AD7768_REG_DIGITAL_FILTER */ -#define AD7768_DIG_FIL_FIL_MSK GENMASK(6, 4) +#define AD7768_DIG_FIL_FIL_MSK GENMASK(7, 4)#define AD7768_DIG_FIL_FIL (x) FIELD_PREP(AD7768_DIG_FIL_FIL_MSK, x)#define AD7768_DIG_FIL_DEC_MSK GENMASK(2, 0)#define AD7768_DIG_FIL_DEC_RATE (x) FIELD_PREP(AD7768_DIG_FIL_DEC_MSK, x) +/* AD7768_SINC3_DEC_RATE */ +#define AD7768_SINC3_DEC_RATE_MSB_MSK GENMASK(12, 8) +#define AD7768_SINC3_DEC_RATE_LSB_MSK GENMASK(7, 0) +/* AD7768_REG_CONVERSION */ #define AD7768_CONV_MODE_MSK GENMASK(2, 0)#define AD7768_CONV_MODE (x) FIELD_PREP(AD7768_CONV_MODE_MSK, x)
@@ -126,22 +131,20 @@ enum ad7768_mclk_div {AD7768_MCLK_DIV_2 }; -enum ad7768_dec_rate { -AD7768_DEC_RATE_32 =0 , -AD7768_DEC_RATE_64 =1 , -AD7768_DEC_RATE_128 =2 , -AD7768_DEC_RATE_256 =3 , -AD7768_DEC_RATE_512 =4 , -AD7768_DEC_RATE_1024 =5 , -AD7768_DEC_RATE_8 =9 , -AD7768_DEC_RATE_16 =10 +enum ad7768_filter_type { +AD7768_FILTER_SINC5 , +AD7768_FILTER_SINC3 , +AD7768_FILTER_WIDEBAND , +AD7768_FILTER_SINC3_REJ60 , }; -struct ad7768_clk_configuration { -enum ad7768_mclk_div mclk_div; -enum ad7768_dec_rate dec_rate; -unsigned int clk_div; -enum ad7768_pwrmode pwrmode; +enum ad7768_filter_regval { +AD7768_FILTER_REGVAL_SINC5 =0 , +AD7768_FILTER_REGVAL_SINC5_X8 =1 , +AD7768_FILTER_REGVAL_SINC5_X16 =2 , +AD7768_FILTER_REGVAL_SINC3 =3 , +AD7768_FILTER_REGVAL_WIDEBAND =4 , +AD7768_FILTER_REGVAL_SINC3_REJ60 =11 , };enum ad7768_scan_type {
@@ -153,18 +156,39 @@ static const int ad7768_mclk_div_rates[4] = { 16, 8, 4, 2, }; -static const struct ad7768_clk_configuration ad7768_clk_config[] = { - {AD7768_MCLK_DIV_2 ,AD7768_DEC_RATE_8 ,16 ,AD7768_FAST_MODE }, - {AD7768_MCLK_DIV_2 ,AD7768_DEC_RATE_16 ,32 ,AD7768_FAST_MODE }, - {AD7768_MCLK_DIV_2 ,AD7768_DEC_RATE_32 ,64 ,AD7768_FAST_MODE }, - {AD7768_MCLK_DIV_2 ,AD7768_DEC_RATE_64 ,128 ,AD7768_FAST_MODE }, - {AD7768_MCLK_DIV_2 ,AD7768_DEC_RATE_128 ,256 ,AD7768_FAST_MODE }, - {AD7768_MCLK_DIV_4 ,AD7768_DEC_RATE_128 ,512 ,AD7768_MED_MODE }, - {AD7768_MCLK_DIV_4 ,AD7768_DEC_RATE_256 ,1024 ,AD7768_MED_MODE }, - {AD7768_MCLK_DIV_4 ,AD7768_DEC_RATE_512 ,2048 ,AD7768_MED_MODE }, - {AD7768_MCLK_DIV_4 ,AD7768_DEC_RATE_1024 ,4096 ,AD7768_MED_MODE }, - {AD7768_MCLK_DIV_8 ,AD7768_DEC_RATE_1024 ,8192 ,AD7768_MED_MODE }, - {AD7768_MCLK_DIV_16 ,AD7768_DEC_RATE_1024 ,16384 ,AD7768_ECO_MODE }, +static const int ad7768_dec_rate_values[8 ] = { +8 ,16 ,32 ,64 ,128 ,256 ,512 ,1024 , +}; + +/* Decimation Rate range for each filter type */ +static const int ad7768_dec_rate_range[][3 ] = { + [AD7768_FILTER_SINC5 ] = {8 ,8 ,1024 }, + [AD7768_FILTER_SINC3 ] = {32 ,32 ,163840 }, + [AD7768_FILTER_WIDEBAND ] = {32 ,32 ,1024 }, + [AD7768_FILTER_SINC3_REJ60 ] = {32 ,32 ,163840 }, +}; + +/* +* The AD7768-1 supports three primary filter types: +* Sinc5, Sinc3, and Wideband. +* However, the filter register values can also encode additional parameters +* such as decimation rates and 60Hz rejection. This utility function separates +* the filter type from these parameters. +*/ +static const int ad7768_filter_regval_to_type[] = { + [AD7768_FILTER_REGVAL_SINC5 ] =AD7768_FILTER_SINC5 , + [AD7768_FILTER_REGVAL_SINC5_X8 ] =AD7768_FILTER_SINC5 , + [AD7768_FILTER_REGVAL_SINC5_X16 ] =AD7768_FILTER_SINC5 , + [AD7768_FILTER_REGVAL_SINC3 ] =AD7768_FILTER_SINC3 , + [AD7768_FILTER_REGVAL_WIDEBAND ] =AD7768_FILTER_WIDEBAND , + [AD7768_FILTER_REGVAL_SINC3_REJ60 ] =AD7768_FILTER_SINC3_REJ60 , +}; + +static const char *const ad7768_filter_enum[] = { + [AD7768_FILTER_SINC5 ] ="sinc5" , + [AD7768_FILTER_SINC3 ] ="sinc3" , + [AD7768_FILTER_WIDEBAND ] ="wideband" , + [AD7768_FILTER_SINC3_REJ60 ] ="sinc3+rej60" };static const struct iio_scan_type ad7768_scan_type[] = {
@@ -183,13 +207,34 @@ static const struct iio_scan_type ad7768_scan_type[] = { }, }; +static int ad7768_get_fil_type_attr (struct iio_dev *dev, +const struct iio_chan_spec *chan); +static int ad7768_set_fil_type_attr (struct iio_dev *dev, +const struct iio_chan_spec *chan,unsigned int filter); + +static const struct iio_enum ad7768_flt_type_iio_enum = { + .items = ad7768_filter_enum, + .num_items =ARRAY_SIZE (ad7768_filter_enum), + .set = ad7768_set_fil_type_attr, + .get = ad7768_get_fil_type_attr, +}; + +static struct iio_chan_spec_ext_info ad7768_ext_info[] = { +IIO_ENUM ("filter_type" ,IIO_SHARED_BY_ALL , &ad7768_flt_type_iio_enum), +IIO_ENUM_AVAILABLE ("filter_type" ,IIO_SHARED_BY_ALL , &ad7768_flt_type_iio_enum), + { } +}; +static const struct iio_chan_spec ad7768_channels[] = { { .type =IIO_VOLTAGE , .info_mask_separate =BIT (IIO_CHAN_INFO_RAW ), - .info_mask_shared_by_type =BIT (IIO_CHAN_INFO_SCALE ), + .info_mask_shared_by_type =BIT (IIO_CHAN_INFO_SCALE ) | +BIT (IIO_CHAN_INFO_OVERSAMPLING_RATIO ), + .info_mask_shared_by_type_available =BIT (IIO_CHAN_INFO_OVERSAMPLING_RATIO ), .info_mask_shared_by_all =BIT (IIO_CHAN_INFO_SAMP_FREQ ), .info_mask_shared_by_all_available =BIT (IIO_CHAN_INFO_SAMP_FREQ ), + .ext_info = ad7768_ext_info, .indexed =1 , .channel =0 , .scan_index =0 ,
@@ -209,9 +254,12 @@ struct ad7768_state {struct clk *mclk;struct gpio_chip gpiochip;unsigned int mclk_freq; -unsigned int dec_rate; +unsigned int mclk_div; +unsigned int oversampling_ratio; +enum ad7768_filter_type filter_type;unsigned int samp_freq; -unsigned int samp_freq_avail[ARRAY_SIZE (ad7768_clk_config)]; +unsigned int samp_freq_avail[ARRAY_SIZE (ad7768_mclk_div_rates)]; +unsigned int samp_freq_avail_len;struct completion completion;struct iio_trigger *trig;struct gpio_desc *gpio_sync_in;
@@ -318,11 +366,35 @@ static int ad7768_send_sync_pulse(struct ad7768_state *st)static void ad7768_fill_samp_freq_tbl (struct ad7768_state *st) { -int i; +int i, freq_filtered, len =0 ; + + freq_filtered =DIV_ROUND_CLOSEST (st->mclk_freq, st->oversampling_ratio); +for (i =0 ; i <ARRAY_SIZE (ad7768_mclk_div_rates); i++) { + st->samp_freq_avail[len] =DIV_ROUND_CLOSEST (freq_filtered, + ad7768_mclk_div_rates[i]); +/* Sampling frequency cannot be lower than the minimum of 50 SPS */ +if (st->samp_freq_avail[len] >=50 ) + len++; + } + st->samp_freq_avail_len = len; +} + +static int ad7768_set_mclk_div (struct ad7768_state *st,unsigned int mclk_div) +{ +unsigned int mclk_div_value; + + mclk_div_value =AD7768_PWR_MCLK_DIV (mclk_div); +/* +* Set power mode based on mclk_div value. +* ECO_MODE is only recommended for MCLK_DIV 16 +*/ + mclk_div_value |= mclk_div >AD7768_MCLK_DIV_16 ? +AD7768_PWR_PWRMODE (AD7768_FAST_MODE ) : +AD7768_PWR_PWRMODE (AD7768_ECO_MODE ); -for (i =0 ; i <ARRAY_SIZE (ad7768_clk_config); i++) - st->samp_freq_avail[i] =DIV_ROUND_CLOSEST (st->mclk_freq, - ad7768_clk_config[i].clk_div); +return regmap_update_bits (st->regmap,AD7768_REG_POWER_CLOCK , +AD7768_PWR_MCLK_DIV_MSK |AD7768_PWR_PWRMODE_MSK , + mclk_div_value); } static int ad7768_set_mode(struct ad7768_state *st,
@@ -358,7 +430,7 @@ static int ad7768_scan_direct(struct iio_dev *indio_dev)* register provides 24-bit data, the precision is reduced by * right-shifting the read value by 8 bits. */ -if (st->dec_rate ==8 ) +if (st->oversampling_ratio ==8 ) readval >>=8 ;/*
@@ -405,22 +477,103 @@ static int ad7768_reg_access(struct iio_dev *indio_dev,return ret; } -static int ad7768_set_dig_fil (struct ad7768_state *st, -enum ad7768_dec_rate dec_rate) +static int ad7768_set_sinc3_dec_rate (struct ad7768_state *st, +unsigned int dec_rate) { -unsigned int mode; +unsigned int max_dec_rate; +u8 dec_rate_reg[2 ];int ret; -if (dec_rate ==AD7768_DEC_RATE_8 || dec_rate ==AD7768_DEC_RATE_16 ) - mode =AD7768_DIG_FIL_FIL (dec_rate); -else - mode =AD7768_DIG_FIL_DEC_RATE (dec_rate); +/* +* Maximum dec_rate is limited by the MCLK_DIV value +* and by the ODR. The edge case is for MCLK_DIV = 2 +* ODR = 50 SPS. +* max_dec_rate <= MCLK / (2 * 50) +*/ + max_dec_rate = st->mclk_freq /100 ; + dec_rate = clamp_t(unsigned int, dec_rate, 32, max_dec_rate); +/* +* Calculate the equivalent value to sinc3 decimation ratio +* to be written on the SINC3_DECIMATION_RATE register: +* Value = (DEC_RATE / 32) -1 +*/ + dec_rate =DIV_ROUND_UP (dec_rate,32 ) -1 ; + dec_rate_reg[0 ] =FIELD_GET (AD7768_SINC3_DEC_RATE_MSB_MSK , dec_rate); + dec_rate_reg[1 ] =FIELD_GET (AD7768_SINC3_DEC_RATE_LSB_MSK , dec_rate); + ret =regmap_bulk_write (st->regmap,AD7768_REG_SINC3_DEC_RATE_MSB , + dec_rate_reg,2 ); +if (ret) +return ret; - ret =regmap_write (st->regmap,AD7768_REG_DIGITAL_FILTER , mode); -if (ret <0 ) + st->oversampling_ratio = (dec_rate +1 ) *32 ; + +return 0 ; +} + +static int ad7768_configure_dig_fil (struct iio_dev *dev, +enum ad7768_filter_type filter_type, +unsigned int dec_rate) +{ +struct ad7768_state *st =iio_priv (dev); +unsigned int dec_rate_idx, dig_filter_regval; +int ret; + +switch (filter_type) { +case AD7768_FILTER_SINC3 : + dig_filter_regval =AD7768_DIG_FIL_FIL (AD7768_FILTER_REGVAL_SINC3 ); +break ; +case AD7768_FILTER_SINC3_REJ60 : + dig_filter_regval =AD7768_DIG_FIL_FIL (AD7768_FILTER_REGVAL_SINC3_REJ60 ); +break ; +case AD7768_FILTER_WIDEBAND : +/* Skip decimations 8 and 16, not supported by the wideband filter */ + dec_rate_idx =find_closest (dec_rate, &ad7768_dec_rate_values[2 ], +ARRAY_SIZE (ad7768_dec_rate_values) -2 ); + dig_filter_regval =AD7768_DIG_FIL_FIL (AD7768_FILTER_REGVAL_WIDEBAND ) | +AD7768_DIG_FIL_DEC_RATE (dec_rate_idx); +/* Correct the index offset */ + dec_rate_idx +=2 ; +break ; +case AD7768_FILTER_SINC5 : + dec_rate_idx =find_closest (dec_rate, ad7768_dec_rate_values, +ARRAY_SIZE (ad7768_dec_rate_values)); + +/* +* Decimations 8 (idx 0) and 16 (idx 1) are set in the +* FILTER[6:4] field. The other decimations are set in the +* DEC_RATE[2:0] field, and the idx need to be offsetted by two. +*/ +if (dec_rate_idx ==0 ) + dig_filter_regval =AD7768_DIG_FIL_FIL (AD7768_FILTER_REGVAL_SINC5_X8 ); +else if (dec_rate_idx ==1 ) + dig_filter_regval =AD7768_DIG_FIL_FIL (AD7768_FILTER_REGVAL_SINC5_X16 ); +else + dig_filter_regval =AD7768_DIG_FIL_FIL (AD7768_FILTER_REGVAL_SINC5 ) | +AD7768_DIG_FIL_DEC_RATE (dec_rate_idx -2 ); +break ; + } + + ret =regmap_write (st->regmap,AD7768_REG_DIGITAL_FILTER , dig_filter_regval); +if (ret)return ret; -/* A sync-in pulse is required every time the filter dec rate changes */ + st->filter_type = filter_type; +/* +* The decimation for SINC3 filters are configured in different +* registers +*/ +if (filter_type ==AD7768_FILTER_SINC3 || + filter_type ==AD7768_FILTER_SINC3_REJ60 ) { + ret =ad7768_set_sinc3_dec_rate (st, dec_rate); +if (ret) +return ret; + }else { + st->oversampling_ratio = ad7768_dec_rate_values[dec_rate_idx]; + } + +ad7768_fill_samp_freq_tbl (st); + +/* A sync-in pulse is required after every configuration change */ return ad7768_send_sync_pulse (st); }
@@ -541,43 +694,69 @@ static int ad7768_gpio_init(struct iio_dev *indio_dev)static int ad7768_set_freq (struct ad7768_state *st,unsigned int freq) { -unsigned int diff_new, diff_old, pwr_mode, i, idx; +unsigned int diff_new, diff_old, i, idx;int res, ret; + freq = clamp_t(unsigned int, freq, 50, 1024000); diff_old =U32_MAX ; idx =0 ; - res =DIV_ROUND_CLOSEST (st->mclk_freq, freq); +if (freq ==0 ) +return -EINVAL ; + + res =DIV_ROUND_CLOSEST (st->mclk_freq, freq * st->oversampling_ratio);/* Find the closest match for the desired sampling frequency */ -for (i =0 ; i <ARRAY_SIZE (ad7768_clk_config); i++) { - diff_new =abs (res - ad7768_clk_config[i].clk_div); +for (i =0 ; i <ARRAY_SIZE (ad7768_mclk_div_rates); i++) { + diff_new =abs (res - ad7768_mclk_div_rates[i]);if (diff_new < diff_old) { diff_old = diff_new; idx = i; } } -/* -* Set both the mclk_div and pwrmode with a single write to the -* POWER_CLOCK register -*/ - pwr_mode =AD7768_PWR_MCLK_DIV (ad7768_clk_config[idx].mclk_div) | -AD7768_PWR_PWRMODE (ad7768_clk_config[idx].pwrmode); - ret =regmap_write (st->regmap,AD7768_REG_POWER_CLOCK , pwr_mode); -if (ret <0 ) +/* Set both the mclk_div and pwrmode */ + ret =ad7768_set_mclk_div (st, idx); +if (ret)return ret; - ret =ad7768_set_dig_fil (st, ad7768_clk_config[idx].dec_rate); -if (ret <0 ) + st->samp_freq =DIV_ROUND_CLOSEST (st->mclk_freq, + ad7768_mclk_div_rates[idx] * st->oversampling_ratio); + +/* A sync-in pulse is required after every configuration change */ +return ad7768_send_sync_pulse (st); +} + +static int ad7768_set_fil_type_attr (struct iio_dev *dev, +const struct iio_chan_spec *chan, +unsigned int filter) +{ +struct ad7768_state *st =iio_priv (dev); +int ret; + + ret =ad7768_configure_dig_fil (dev, filter, st->oversampling_ratio); +if (ret)return ret; - st->dec_rate = ad7768_clk_config[idx].clk_div / - ad7768_mclk_div_rates[ad7768_clk_config[idx].mclk_div]; - st->samp_freq =DIV_ROUND_CLOSEST (st->mclk_freq, - ad7768_clk_config[idx].clk_div); +/* Update sampling frequency */ +return ad7768_set_freq (st, st->samp_freq); +} -return 0 ; +static int ad7768_get_fil_type_attr (struct iio_dev *dev, +const struct iio_chan_spec *chan) +{ +struct ad7768_state *st =iio_priv (dev); +int ret; +unsigned int mode; + + ret =regmap_read (st->regmap,AD7768_REG_DIGITAL_FILTER , &mode); +if (ret) +return ret; + + mode =FIELD_GET (AD7768_DIG_FIL_FIL_MSK , mode); + +/* From the register value, get the corresponding filter type */ +return ad7768_filter_regval_to_type[mode]; } static int ad7768_read_raw(struct iio_dev *indio_dev,
@@ -619,6 +798,11 @@ static int ad7768_read_raw(struct iio_dev *indio_dev,case IIO_CHAN_INFO_SAMP_FREQ : *val = st->samp_freq; +return IIO_VAL_INT ; + +case IIO_CHAN_INFO_OVERSAMPLING_RATIO : + *val = st->oversampling_ratio; +return IIO_VAL_INT ; }
@@ -633,9 +817,13 @@ static int ad7768_read_avail(struct iio_dev *indio_dev,struct ad7768_state *st =iio_priv (indio_dev);switch (info) { +case IIO_CHAN_INFO_OVERSAMPLING_RATIO : + *vals = (int *)ad7768_dec_rate_range[st->filter_type]; + *type =IIO_VAL_INT ; +return IIO_AVAIL_RANGE ;case IIO_CHAN_INFO_SAMP_FREQ : *vals = (int *)st->samp_freq_avail; - *length =ARRAY_SIZE (ad7768_clk_config); + *length = st->samp_freq_avail_len; *type =IIO_VAL_INT ;return IIO_AVAIL_LIST ;default :
@@ -643,20 +831,44 @@ static int ad7768_read_avail(struct iio_dev *indio_dev, } } -static int ad7768_write_raw (struct iio_dev *indio_dev, -struct iio_chan_spec const *chan, -int val,int val2,long info) +static int __ad7768_write_raw (struct iio_dev *indio_dev, +struct iio_chan_spec const *chan, +int val,int val2,long info) {struct ad7768_state *st =iio_priv (indio_dev); +int ret;switch (info) {case IIO_CHAN_INFO_SAMP_FREQ :return ad7768_set_freq (st, val); + +case IIO_CHAN_INFO_OVERSAMPLING_RATIO : + ret =ad7768_configure_dig_fil (indio_dev, st->filter_type, val); +if (ret) +return ret; + +/* Update sampling frequency */ +return ad7768_set_freq (st, st->samp_freq);default :return -EINVAL ; } } +static int ad7768_write_raw (struct iio_dev *indio_dev, +struct iio_chan_spec const *chan, +int val,int val2,long info) +{ +int ret; + +if (!iio_device_claim_direct (indio_dev)) +return -EBUSY ; + + ret =__ad7768_write_raw (indio_dev, chan, val, val2, info); +iio_device_release_direct (indio_dev); + +return ret; +} +static int ad7768_read_label (struct iio_dev *indio_dev,const struct iio_chan_spec *chan,char *label) {
@@ -670,7 +882,7 @@ static int ad7768_get_current_scan_type(const struct iio_dev *indio_dev, {struct ad7768_state *st =iio_priv (indio_dev); -return st->dec_rate ==8 ?AD7768_SCAN_TYPE_HIGH_SPEED : +return st->oversampling_ratio ==8 ?AD7768_SCAN_TYPE_HIGH_SPEED :AD7768_SCAN_TYPE_NORMAL ; }
@@ -793,6 +1005,14 @@ static int ad7768_setup(struct iio_dev *indio_dev)return ret; } +/* +* Set Default Digital Filter configuration: +* SINC5 filter with x32 Decimation rate +*/ + ret =ad7768_configure_dig_fil (indio_dev,AD7768_FILTER_SINC5 ,32 ); +if (ret) +return ret; +/* Set the default sampling frequency to 32000 kSPS */ return ad7768_set_freq (st,32000 ); }
@@ -1143,7 +1363,6 @@ static int ad7768_probe(struct spi_device *spi)return PTR_ERR (st->mclk); st->mclk_freq =clk_get_rate (st->mclk); -ad7768_fill_samp_freq_tbl (st); indio_dev->channels = ad7768_channels; indio_dev->num_channels =ARRAY_SIZE (ad7768_channels);
Signature · 1 line
2.34.1
Re: [PATCH v5 13/14] iio: adc: ad7768-1: add filter type and oversampling ratio attributes
Quoted 11 lines · Jonathan Santos
Separate filter type and decimation rate from the sampling frequency attribute. The new filter type attribute enables sinc3, sinc3+rej60 and wideband filters, which were previously unavailable.
Previously, combining decimation and MCLK divider in the sampling frequency obscured performance trade-offs. Lower MCLK divider settings increase power usage, while lower decimation rates reduce precision by decreasing averaging. By creating an oversampling attribute, which controls the decimation, users gain finer control over performance.
Quoted 3 lines
The addition of those attributes allows a wider range of sampling frequencies and more access to the device features. Sampling frequency table is updated after every digital filter paramerter change.
s/paramerter/parameter/
Quoted 5 lines
Co-developed-by Pop Paul <[email protected]>Signed-off-by Pop Paul <[email protected]>Signed-off-by Jonathan Santos <[email protected]>---
The hardware designers sure didn't make this one easy for us. I'm very impressed that you were able to write code that actually makes sense to me without me having to read the data sheet 10 times. Nice work! :-)
A few minor things to fix, but otherwise...
drivers/iio/adc/ad7768-1.c static const int ad7768_mclk_div_rates[4] = { · line 158
Quoted 3 lines
drivers/iio/adc/ad7768-1.c +0
-3-static const struct ad7768_clk_configuration ad7768_clk_config[] = { - {AD7768_MCLK_DIV_2 ,AD7768_DEC_RATE_8 ,16 ,AD7768_FAST_MODE }, - {AD7768_MCLK_DIV_2 ,AD7768_DEC_RATE_16 ,32 ,AD7768_FAST_MODE },
Extra spaces should be removed.
Quoted 12 lines
drivers/iio/adc/ad7768-1.c +3
-9- { AD7768_MCLK_DIV_2, AD7768_DEC_RATE_32, 64, AD7768_FAST_MODE }, - { AD7768_MCLK_DIV_2, AD7768_DEC_RATE_64, 128, AD7768_FAST_MODE }, - { AD7768_MCLK_DIV_2, AD7768_DEC_RATE_128, 256, AD7768_FAST_MODE }, - { AD7768_MCLK_DIV_4, AD7768_DEC_RATE_128, 512, AD7768_MED_MODE }, - { AD7768_MCLK_DIV_4, AD7768_DEC_RATE_256, 1024, AD7768_MED_MODE }, - { AD7768_MCLK_DIV_4, AD7768_DEC_RATE_512, 2048, AD7768_MED_MODE }, - { AD7768_MCLK_DIV_4, AD7768_DEC_RATE_1024, 4096, AD7768_MED_MODE }, - { AD7768_MCLK_DIV_8, AD7768_DEC_RATE_1024, 8192, AD7768_MED_MODE }, - { AD7768_MCLK_DIV_16, AD7768_DEC_RATE_1024, 16384, AD7768_ECO_MODE }, +static const int ad7768_dec_rate_values[8 ] = { +8 ,16 ,32 ,64 ,128 ,256 ,512 ,1024 , +};
...
Quoted 5 lines
+/*
+ * The AD7768-1 supports three primary filter types:
+ * Sinc5, Sinc3, and Wideband.
+ * However, the filter register values can also encode additional parameters
+ * such as decimation rates and 60Hz rejection. This utility function separates
Technically, this is an array, not a function.
Quoted 2 lines
+ * the filter type from these parameters.
+ */
...
drivers/iio/adc/ad7768-1.c static const struct iio_scan_type ad7768_scan_type[] = { · line 219
Quoted 5 lines
drivers/iio/adc/ad7768-1.c +5
-0+static const struct iio_enum ad7768_flt_type_iio_enum = { + .items = ad7768_filter_enum, + .num_items =ARRAY_SIZE (ad7768_filter_enum), + .set = ad7768_set_fil_type_attr, + .get = ad7768_get_fil_type_attr,
Can we spell out filter here? It took me quite a while to figure out what "fil" is.
drivers/iio/adc/ad7768-1.c static const struct iio_scan_type ad7768_scan_type[] = { · line 224
Quoted 5 lines
drivers/iio/adc/ad7768-1.c +5
-0+}; + +static struct iio_chan_spec_ext_info ad7768_ext_info[] = { +IIO_ENUM ("filter_type" ,IIO_SHARED_BY_ALL , &ad7768_flt_type_iio_enum), +IIO_ENUM_AVAILABLE ("filter_type" ,IIO_SHARED_BY_ALL , &ad7768_flt_type_iio_enum),
"flt" is a bit more obvious, but still would be nice to spell it out too.
Quoted 3 lines
drivers/iio/adc/ad7768-1.c +3
-0+ { } +}; +
Jonathan Cameron
maintainerRe: [PATCH v5 13/14] iio: adc: ad7768-1: add filter type and oversampling ratio attributes
In reply to Jonathan SantosQuoted 23 lines
drivers/iio/adc/ad7768-1.c +10
-7-static int ad7768_set_dig_fil (struct ad7768_state *st, -enum ad7768_dec_rate dec_rate) +static int ad7768_set_sinc3_dec_rate (struct ad7768_state *st, +unsigned int dec_rate) { -unsigned int mode; +unsigned int max_dec_rate; +u8 dec_rate_reg[2 ];int ret; -if (dec_rate ==AD7768_DEC_RATE_8 || dec_rate ==AD7768_DEC_RATE_16 ) - mode =AD7768_DIG_FIL_FIL (dec_rate); -else - mode =AD7768_DIG_FIL_DEC_RATE (dec_rate); +/* +* Maximum dec_rate is limited by the MCLK_DIV value +* and by the ODR. The edge case is for MCLK_DIV = 2 +* ODR = 50 SPS. +* max_dec_rate <= MCLK / (2 * 50) +*/
drivers/iio/adc/ad7768-1.c static int ad7768_reg_access(struct iio_dev *indio_dev, · line 502
Quoted 10 lines
drivers/iio/adc/ad7768-1.c +10
-0+ max_dec_rate = st->mclk_freq /100 ; + dec_rate = clamp_t(unsigned int, dec_rate, 32, max_dec_rate); +/* +* Calculate the equivalent value to sinc3 decimation ratio +* to be written on the SINC3_DECIMATION_RATE register: +* Value = (DEC_RATE / 32) -1 +*/ + dec_rate =DIV_ROUND_UP (dec_rate,32 ) -1 ; + dec_rate_reg[0 ] =FIELD_GET (AD7768_SINC3_DEC_RATE_MSB_MSK , dec_rate); + dec_rate_reg[1 ] =FIELD_GET (AD7768_SINC3_DEC_RATE_LSB_MSK , dec_rate);
Looks like a larger big endian value. It's a little messy because of the 12 bit mask but I think still clearer as
u16 regval = FIELD_PREP(GENMASK(11, 0), dec_rate); unaligned_put_be16(dec_rate_reg, regval);
Avoids the use of masks to get bytes from the dec_rate value which is is sort of backwards.
Quoted 5 lines
drivers/iio/adc/ad7768-1.c +4
-0+ ret =regmap_bulk_write (st->regmap,AD7768_REG_SINC3_DEC_RATE_MSB , + dec_rate_reg,2 ); +if (ret) +return ret;
PATCH v5 14/14 iio: adc: ad7768-1: add low pass -3dB cutoff attribute
In reply to Jonathan SantosAd7768-1 has a different -3db frequency multiplier depending on the filter type configured. The cutoff frequency also varies according to the current ODR.
Add a readonly low pass -3dB frequency cutoff attribute to clarify to the user which bandwidth is being allowed depending on the filter configurations.
---
v5 Changes:
* None
v4 Changes:
* None
v3 Changes:
* None
v2 Changes:
* New patch in v2.
--- drivers/iio/adc/ad7768-1.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-)
drivers/iio/adc/ad7768-1.c +20 -2
@@ -152,6 +152,17 @@ enum ad7768_scan_type { AD7768_SCAN_TYPE_HIGH_SPEED, }; +/* +* -3dB cutoff frequency multipliers (relative to ODR) for +* each filter type. Values are multiplied by 1000. +*/ +static const int ad7768_filter_3db_odr_multiplier[] = { + [AD7768_FILTER_SINC5 ] =204 , + [AD7768_FILTER_SINC3 ] =262 , + [AD7768_FILTER_SINC3_REJ60 ] =262 , + [AD7768_FILTER_WIDEBAND ] =433 , +}; +static const int ad7768_mclk_div_rates[4 ] = {16 ,8 ,4 ,2 , };
@@ -230,7 +241,8 @@ static const struct iio_chan_spec ad7768_channels[] = { .type = IIO_VOLTAGE, .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), .info_mask_shared_by_type =BIT (IIO_CHAN_INFO_SCALE ) | - BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), +BIT (IIO_CHAN_INFO_OVERSAMPLING_RATIO ) | + BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY), .info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), .info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
@@ -765,7 +777,7 @@ static int ad7768_read_raw(struct iio_dev *indio_dev, {struct ad7768_state *st =iio_priv (indio_dev);const struct iio_scan_type *scan_type; -int scale_uv, ret; +int scale_uv, ret, temp; scan_type =iio_get_current_scan_type (indio_dev, chan);if (IS_ERR (scan_type))
@@ -803,6 +815,12 @@ static int ad7768_read_raw(struct iio_dev *indio_dev,case IIO_CHAN_INFO_OVERSAMPLING_RATIO : *val = st->oversampling_ratio; +return IIO_VAL_INT ; + +case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY : + temp = st->samp_freq * ad7768_filter_3db_odr_multiplier[st->filter_type]; + *val =DIV_ROUND_CLOSEST (temp,1000 ); +return IIO_VAL_INT ; }
Signature · 1 line
2.34.1