Add support for AD4630 common mode data #2597
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This gives the channel scan_type a named type so that it can be used to simplify code in later commits. Signed-off-by: David Lechner <[email protected]>
By using struct iio_scan_type, we can simplify the code by removing lots of duplicate pointer dereferences. This make the code a bit easier to read. This also prepares for a future where channels may have more than one scan_type. Signed-off-by: David Lechner <[email protected]>
This adds new fields to the iio_channel structure to support multiple scan types per channel. This is useful for devices that support multiple resolution modes or other modes that require different data formats of the raw data. To make use of this, drivers need to implement the new callback get_current_scan_type() to resolve the scan type for a given channel based on the current state of the driver. There is a new scan_type_ext field in the iio_channel structure that should be used to store the scan types for any channel that has more than one. There is also a new flag has_ext_scan_type that acts as a type discriminator for the scan_type/ext_scan_type union. A union is used so that we don't grow the size of the iio_channel structure and also makes it clear that scan_type and ext_scan_type are mutually exclusive. The buffer code is the only code in the IIO core code that is using the scan_type field. This patch updates the buffer code to use the new iio_channel_validate_scan_type() function to ensure it is returning the correct scan type for the current state of the device when reading the sysfs attributes. The buffer validation code is also update to validate any additional scan types that are set in the scan_type_ext field. Part of that code is refactored to a new function to avoid duplication. Some userspace tools may need to be updated to re-read the scan type after writing any other attribute. During testing, we noticed that we had to restart iiod to get it to re-read the scan type after enabling oversampling on the ad7380 driver. Signed-off-by: David Lechner <[email protected]>
AD4030, AD4630, ADAQ4224, and similar devices can append an 8-bit code representing the input common-mode voltage to the 16-bit or 24-bit ADC conversion output code. Those 8-bit common mode code come after the conversion bits in the same SPI transfer. Thus, when common-mode output is enabled, every conversion brings common mode bits and common mode bits can only be read by doing an ADC conversion. Because ADC conversion bits and common mode bits are put together into buffer scan elements, user space applications can't properly handle data from IIO buffers since it only has info about one data element (either ADC conversion or common mode bits). The number of shift bits and the amount of realbits are different for when common-mode output is disabled and for when it's enabled. Add iio_scan_type structs to tell user space different parameters for element shift and realbits so applications can handle buffer elements properly when common mode bits are enabled. Signed-off-by: Marcelo Schmitt <[email protected]>
|
Hi @machschmitt , Is there any special request for this? Note that this driver is being upstreamed and the common mode bits are being proposed as an extra channel. Hence, if there's no urgency, I would say to drop this and wait for the upstreamed version which will be the one we want to use (naturally we'll need to add out of tree spi_offload support for now). |
|
Hi @nunojsa, quick update about this: I explained the current AD4030/AD4630/ADAQ4224 development status (including the upstreaming part of it) to BU guys and the additional effort it would require to have this temporary feature in ADI main. I then asked BU guys about the urgency of the common-mode feature but got no reply so far. Waiting to see what they decide. Hope we can be saved from having more work by waiting for upstream ad4030 driver :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Description
This PR updates ad4630 so it can provide ADC common-mode data.
First, this PR brings multiple scan_type support from upstream into ADI Linux (https://lore.kernel.org/linux-iio/20240530-iio-add-support-for-multiple-scan-types-v3-0-cbc4acea2cfa@baylibre.com/).
Then, ad4630 is extended to provide one scan_type to read ADC conversion, one scan_type to read common-mode voltage, and another scan_type to read both ADC conversion and common-mode voltage.
This feature request was written in this JIRA task: https://jira.analog.com/browse/COSGTM-966.
After doing a self-review of the code, I don't really think this is a good solution for having the common-mode voltage. I think that common-mode voltage should be provided as a separate IIO channel, specially because common-mode LSB is VREF/256 (would need a separate scale attribute). Though, this is the less worse solution I could come up with so proposing it here for comments.
PR Type
PR Checklist