Skip to content

Conversation

@dNechita
Copy link
Contributor

PR Description

The problem:

The scan-element format (e.g. le:S24/32>>0 vs le:S20/32>>0) can change at runtime when users modify oversampling / resolution attributes. Libiio caches the original type string at context creation time with no means of changing it afterwards.
This could affect downstream size calculations and conversion logic.

The fix:

  • Add public API: iio_channel_refresh_data_format() - explicit manual refresh
  • A refresh on all relevant channels is done right before buffer enable. This guarantees format correctness at the moment streaming starts.

Note: this implementation targets only the local backend. The rest of the backends need to be updated as well.

PR Type

  • Bug fix (a change that fixes an issue)
  • New feature (a change that adds new functionality)
  • Breaking change (a change that affects other repos or cause CIs to fail)

PR Checklist

  • I have conducted a self-review of my own code changes
  • I have commented new code, particularly complex or unclear areas
  • I have checked that I did not introduce new warnings or errors (CI output)
  • I have checked that components that use libiio did not get broken
  • I have updated the documentation accordingly (GitHub Pages, READMEs, etc)

The problem:

The scan-element format (e.g. le:S24/32>>0 vs le:S20/32>>0) can change
at runtime when users modify oversampling / resolution attributes.
Libiio caches the original type string at context creation time with no
means of changing it.
This could affect downstream size calculations and conversion logic.

The fix:
 - Add public API: iio_channel_refresh_data_format() - explicit manual
refresh
 - A refresh on all relevant channels is done right before buffer
enable. This guarantees format correctness at the moment
streaming starts.

Note: this implementation targets only the local backend.
The rest of the backends need to be updated as well.

Signed-off-by: Dan Nechita <[email protected]>
@rgetz
Copy link
Contributor

rgetz commented Nov 15, 2025

rather than forcing this logic into all the applications - (refreshing things) - why not just do this inside libiio? (ie. - for this one special one - don't cache it).

@gastmaier
Copy link
Contributor

gastmaier commented Nov 16, 2025

When I was looking into it I was trying to make scan_type an attribute, so it would be read like the others (e.g., _raw)
And then I tripped into another design choices.
For example, take this device:

$ tree
.
├── buffer
│   ├── data_available
│   ├── direction
│   ├── enable
│   ├── length
│   └── watermark
├── buffer0
│   ├── data_available
│   ├── direction
│   ├── enable
│   ├── in_voltage0_en
│   ├── in_voltage0_index
│   ├── in_voltage0_type
│   ├── length
│   └── watermark
├── events
│   ├── sampling_frequency
│   ├── sampling_frequency_available
│   ├── thresh_either_en
│   ├── thresh_falling_hysteresis
│   ├── thresh_falling_value
│   ├── thresh_rising_hysteresis
│   └── thresh_rising_value
├── in_voltage_calibscale
├── in_voltage_oversampling_ratio
├── in_voltage_oversampling_ratio_available
├── in_voltage_raw
├── in_voltage_scale
├── name
├── sampling_frequency
├── sampling_frequency_available
├── scan_elements
│   ├── in_voltage0_en
│   ├── in_voltage0_index
│   └── in_voltage0_type
├── trigger
│   └── current_trigger
├── uevent
└── waiting_for_supplier

Per ABI, it contains a general sampling_frequency and a events/sampling_frequency only for the sample rate used in events, as well as all events attributes are under events.
But when I do

iio_info

        iio:device0: ad4062 (buffer capable)
                1 channels found:
                        voltage0:  (input, index: 0, format: be:s16/32>>0)
                        5 channel-specific attributes found:
                                attr  0: calibscale (in_voltage_calibscale) value: 1.000000000
                                attr  1: oversampling_ratio (in_voltage_oversampling_ratio) value: 1
                                attr  2: oversampling_ratio_available (in_voltage_oversampling_ratio_available) options: 1 2 4 8 16 32 64 128 256 512 1024 2048 4096
                                attr  3: raw (in_voltage_raw) value: -1
                                attr  4: scale (in_voltage_scale) value: 0.201416015
                9 device-specific attributes found:
                        attr  0: sampling_frequency value: 2000000
                        attr  1: sampling_frequency value: 2000000
                        attr  2: sampling_frequency_available options: 2000000 1000000 300000 100000 33300 10000 3000 500 333 250 200 166 140 124 111
                        attr  3: sampling_frequency_available options: 2000000 1000000 300000 100000 33300 10000 3000 500 333 250 200 166 140 124 111
                        attr  4: thresh_either_en value: ERROR: No such file or directory (-2)
                        attr  5: thresh_falling_hysteresis value: ERROR: No such file or directory (-2)
                        attr  6: thresh_falling_value value: ERROR: No such file or directory (-2)
                        attr  7: thresh_rising_hysteresis value: ERROR: No such file or directory (-2)
                        attr  8: thresh_rising_value value: ERROR: No such file or directory (-2)
                2 buffer attributes found:
                        attr  0: data_available options: 128
                        attr  1: direction value: in
                1 debug attributes found:
                        attr  0: direct_reg_access
                Current trigger: trigger0(ad4062-dev0)

The paths (e.g., events) are flushed away, resulting in "duplicated" sampling_frequency, as well as, currently (84aea8d) all events attributes appear to be broken, I guess trying to read from /sys/bus/iio/devices/iio\:device0/thresh_either_en instead of/sys/bus/iio/devices/iio\:device0/events/thresh_either_en.
Kernel version: 6.12.0+

I see in the source code hardcoded "events/", scan_elements/, dir_is_scan_elements ...
Wouldn't be easier to have the attributes names the full relpath events/thresh_either_en, and recursively add all that are not symbolic links (e.g. excludes subsystem, supplier*, of_node)?

I believe this way would be easier to set rules of which should or should not be cached.

$ iio_genxml

<?xml version="1.0" encoding="utf-8"?><!DOCTYPE context [<!ELEMENT context (device | context-attribute)*><!ELEMENT context-attribute EMPTY><!ELEMENT device (channel | attribute | debug-attribute | buffer-attribute)*><!ELEMENT channel (scan-element?, attribute*)><!ELEMENT attribute EMPTY><!ELEMENT scan-element EMPTY><!ELEMENT debug-attribute EMPTY><!ELEMENT buffer-attribute EMPTY><!ATTLIST context name CDATA #REQUIRED version-major CDATA #REQUIRED version-minor CDATA #REQUIRED version-git CDATA #REQUIRED description CDATA #IMPLIED><!ATTLIST context-attribute name CDATA #REQUIRED value CDATA #REQUIRED><!ATTLIST device id CDATA #REQUIRED name CDATA #IMPLIED label CDATA #IMPLIED><!ATTLIST channel id CDATA #REQUIRED type (input|output) #REQUIRED name CDATA #IMPLIED label CDATA #IMPLIED><!ATTLIST scan-element index CDATA #REQUIRED format CDATA #REQUIRED scale CDATA #IMPLIED><!ATTLIST attribute name CDATA #REQUIRED filename CDATA #IMPLIED><!ATTLIST debug-attribute name CDATA #REQUIRED><!ATTLIST buffer-attribute name CDATA #REQUIRED>]><context name="local" version-major="1" version-minor="0" version-git="84aea8dd" description="Linux analog 6.12.0+ #6 SMP PREEMPT Wed Nov 12 22:20:44 CET 2025 armv7l" ><context-attribute name="local,kernel" value="6.12.0+" /><context-attribute name="uri" value="local:" /><device id="iio:device0" name="ad4062" ><channel id="voltage0" type="input" ><scan-element index="0" format="be:s16/32&gt;&gt;0" scale="0.201416" /><attribute name="calibscale" filename="in_voltage_calibscale" /><attribute name="oversampling_ratio" filename="in_voltage_oversampling_ratio" /><attribute name="oversampling_ratio_available" filename="in_voltage_oversampling_ratio_available" /><attribute name="raw" filename="in_voltage_raw" /><attribute name="scale" filename="in_voltage_scale" /></channel><attribute name="sampling_frequency" /><attribute name="sampling_frequency" /><attribute name="sampling_frequency_available" /><attribute name="sampling_frequency_available" /><attribute name="thresh_either_en" /><attribute name="thresh_falling_hysteresis" /><attribute name="thresh_falling_value" /><attribute name="thresh_rising_hysteresis" /><attribute name="thresh_rising_value" /><debug-attribute name="direct_reg_access" /><buffer-attribute name="data_available" /><buffer-attribute name="direction" /></device><device id="trigger0" name="ad4062-dev0" ></device></context>

@nunojsa
Copy link
Contributor

nunojsa commented Nov 17, 2025

rather than forcing this logic into all the applications - (refreshing things) - why not just do this inside libiio? (ie. - for this one special one - don't cache it).

Agree. I guess we could read it (cache it) before enabling buffering. A driver changing oversampling while buffering is most likely a bug in driver code itself. Or we just read it from the device "on demand".

The paths (e.g., events) are flushed away, resulting in "duplicated" sampling_frequency

Yes, that's not good...

scan_elements/, dir_is_scan_elements

Also note that the above directory together with buffer/ is legacy (I guess we still need to be aware of them for older kernels) but if buffer0/ exists, that should be the one we look at.

@dNechita
Copy link
Contributor Author

rather than forcing this logic into all the applications - (refreshing things) - why not just do this inside libiio? (ie. - for this one special one - don't cache it).

Agree. I guess we could read it (cache it) before enabling buffering. A driver changing oversampling while buffering is most likely a bug in driver code itself. Or we just read it from the device "on demand".

Yes, that is exactly what is happening with the current changes, the format type is refreshed exactly before enabling the buffer. This step would be enough for most use cases. However, I wanted to provide users the ability to choose the moment when the caching is done as there might be scenarios where users modify the oversampling and know format might change and can call iio_channel_refresh_data_format() to get the newly format right away, possibly to just display it on their application or do something else with it like initializing some algorithm that uses the new format.

I thought about making iio_channel_get_data_format() always perform a fresh read instead of caching, but this would create significant performance overhead for already existing applications that call this function on every buffer element.

@rgetz
Copy link
Contributor

rgetz commented Nov 21, 2025

Is this just a break in the social contract between the kernel and libiio then? kernel if it wants to have things be dynamic - should be suffixing with _raw?

Maybe @pcercuei or @larsclausen or @mhennerich knows the vision better...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants