Messages in this thread | | | Date | Fri, 29 Sep 2023 17:02:29 -0500 | Subject | Re: [PATCH v3 00/27] iio: resolver: move ad2s1210 out of staging | From | David Lechner <> |
| |
On 9/29/23 12:23 PM, David Lechner wrote: > From: David Lechner <david@lechnology.com> > > v3 changes: > > * Added description of A0/A1 lines in DT bindings. > * Added power supply regulators to DT bindings. > * Dropped "staging: iio: Documentation: document IIO resolver AD2S1210 > sysfs attributes" (these attributes are being removed instead). > * Dropped applied patches: > * "staging: iio: resolver: ad2s1210: fix ad2s1210_show_fault" > * "iio: adc: MCP3564: fix the static checker warning" > * Split "staging: iio: resolver: ad2s1210: fix probe" into multiple patches. > * Moved sorting imports to separate patch. > * Renamed fclkin to clkin_hz. > * Added __be16 sample field to state struct for reading raw samples. > * Split out new function ad2s1210_single_conversion() from > ad2s1210_read_raw(). > * Split out new ad2s1210_get_hysteresis() and ad2s1210_set_hysteresis() > functions. > * Fixed multi-line comment style. > * Added notes about soft reset not resetting config registers. > * Made use of FIELD_PREP() macro. > * Added more explanation to regmap commit message. > * Removed datasheet names from channel specs. > * Replaced "staging: iio: resolver: ad2s1210: rename fexcit attribute" > with "staging: iio: resolver: ad2s1210: convert fexcit to channel > attribute". > * Replaced "staging: iio: resolver: ad2s1210: add phase_lock_range > attributes" with "staging: iio: resolver: ad2s1210: add phase lock > range support" > * Added additional patches to convert custom device attributes to event > attributes. > * Added patch to add channel label attributes. > > v2 changes: > * Address initial device tree patch feedback > * Drop "iio: sysfs: add IIO_DEVICE_ATTR_NAMED_RW macro" (related cleanups > also dropped for now, will address in a future series if needed) > * Apply improvements as a series of patches to the staging driver. It is > not quite ready for the move out of staging patch yet. > > This series has been tested on actual hardware using a EVAL-AD2S1210 evaluation > board. (Note: not all device tree features have been implemented in the driver > since the eval board doesn't support them out of the box. We plan to add them > later if needed.) > > Most of the questions about dealing with faults from the v2 cover letter > have been addressed. There is still the question about what to do with > the current `fault` attribute (it is the only custom device attribute > remaining from the original staging driver). It was suggested to split it > out into multiple attributes in a subdirectory. Since we now have events > for all of the faults, I'm wondering if this is something that is still needed. > In the current implementation, it is possible to start listening to events, > clear the faults and then read a sample to trigger events for any current > faults so we have a way to get current faults already. > > There is also the matter of clearing faults. Writing the excitation > frequency has a side-effect of clearing the faults, so we could use > that as the reset. Or we could change the current fault attribute to > write-only and rename it. Or is there a better way that I have overlooked? > > Once this last issue is addressed, I think this driver will be ready > for consideration for moving out of staging. > --- > David Lechner (27): > dt-bindings: iio: resolver: add devicetree bindings for ad2s1210 > staging: iio: resolver: ad2s1210: fix use before initialization > staging: iio: resolver: ad2s1210: remove call to spi_setup() > staging: iio: resolver: ad2s1210: check return of ad2s1210_initial() > staging: iio: resolver: ad2s1210: remove spi_set_drvdata() > staging: iio: resolver: ad2s1210: sort imports > staging: iio: resolver: ad2s1210: always use 16-bit value for raw read > staging: iio: resolver: ad2s1210: implement IIO_CHAN_INFO_SCALE > staging: iio: resolver: ad2s1210: use devicetree to get CLKIN rate > staging: iio: resolver: ad2s1210: use regmap for config registers > staging: iio: resolver: ad2s1210: add debugfs reg access > staging: iio: resolver: ad2s1210: remove config attribute > staging: iio: resolver: ad2s1210: rework gpios > staging: iio: resolver: ad2s1210: implement hysteresis as channel attr > staging: iio: resolver: ad2s1210: refactor setting excitation frequency > staging: iio: resolver: ad2s1210: read excitation frequency from control register > staging: iio: resolver: ad2s1210: convert fexcit to channel attribute > staging: iio: resolver: ad2s1210: convert resolution to devicetree property > staging: iio: resolver: ad2s1210: add phase lock range support > staging: iio: resolver: ad2s1210: add triggered buffer support > staging: iio: resolver: ad2s1210: convert LOT threshold attrs to event attrs > staging: iio: resolver: ad2s1210: convert LOS threshold to event attr > staging: iio: resolver: ad2s1210: convert DOS overrange threshold to event attr > staging: iio: resolver: ad2s1210: convert DOS mismatch threshold to event attr > staging: iio: resolver: ad2s1210: rename DOS reset min/max attrs > staging: iio: resolver: ad2s1210: implement fault events > staging: iio: resolver: ad2s1210: add label attribute support > > .../bindings/iio/resolver/adi,ad2s1210.yaml | 177 +++ > .../Documentation/sysfs-bus-iio-resolver-ad2s1210 | 27 + > drivers/staging/iio/resolver/Kconfig | 1 + > drivers/staging/iio/resolver/ad2s1210.c | 1583 +++++++++++++++----- > 4 files changed, 1391 insertions(+), 397 deletions(-) > --- > base-commit: 5e99f692d4e32e3250ab18d511894ca797407aec > change-id: 20230925-ad2s1210-mainline-2791ef75e386 >
In the end, this is what sysfs looks like:
root@analog:~# tree /sys/bus/iio/devices/iio\:device1/ /sys/bus/iio/devices/iio:device1/ ├── buffer │ ├── data_available │ ├── direction │ ├── enable │ ├── length │ └── watermark ├── buffer0 │ ├── data_available │ ├── direction │ ├── enable │ ├── in_angl0_en │ ├── in_angl0_index │ ├── in_angl0_type │ ├── in_anglvel0_en │ ├── in_anglvel0_index │ ├── in_anglvel0_type │ ├── in_timestamp_en │ ├── in_timestamp_index │ ├── in_timestamp_type │ ├── length │ └── watermark ├── current_timestamp_clock ├── dev ├── events │ ├── in_altvoltage0_mag_reset_max │ ├── in_altvoltage0_mag_reset_max_available │ ├── in_altvoltage0_mag_reset_min │ ├── in_altvoltage0_mag_reset_min_available │ ├── in_altvoltage0_mag_value │ ├── in_altvoltage0_mag_value_available │ ├── in_altvoltage0_thresh_falling_value │ ├── in_altvoltage0_thresh_falling_value_available │ ├── in_altvoltage0_thresh_rising_value │ ├── in_altvoltage0_thresh_rising_value_available │ ├── in_angl1_thresh_rising_hysteresis │ ├── in_angl1_thresh_rising_hysteresis_available │ ├── in_angl1_thresh_rising_value │ ├── in_angl1_thresh_rising_value_available │ ├── in_phase0_mag_value │ └── in_phase0_mag_value_available ├── fault ├── in_altvoltage0_label ├── in_altvoltage1_x_label ├── in_altvoltage1_y_label ├── in_angl0_hysteresis ├── in_angl0_hysteresis_available ├── in_angl0_label ├── in_angl0_raw ├── in_angl0_scale ├── in_angl1_label ├── in_anglvel0_label ├── in_anglvel0_raw ├── in_anglvel0_scale ├── in_phase0_label ├── name ├── of_node -> ../../../../../../../../firmware/devicetree/base/axi/spi@e0006000/ad2s1210@0 ├── out_altvoltage0_frequency ├── out_altvoltage0_frequency_available ├── out_altvoltage0_label ├── power │ ├── autosuspend_delay_ms │ ├── control │ ├── runtime_active_time │ ├── runtime_status │ └── runtime_suspended_time ├── scan_elements │ ├── in_angl0_en │ ├── in_angl0_index │ ├── in_angl0_type │ ├── in_anglvel0_en │ ├── in_anglvel0_index │ ├── in_anglvel0_type │ ├── in_timestamp_en │ ├── in_timestamp_index │ └── in_timestamp_type ├── subsystem -> ../../../../../../../../bus/iio ├── trigger │ └── current_trigger ├── uevent └── waiting_for_supplier
8 directories, 72 files
And this is what the output of iio_info looks like: (note: iio_info does not currently support events so those attributes are not visible here)
iio:device1: ad2s1210 (buffer capable) 9 channels found: angl0: (input, index: 0, format: be:U16/16>>0) 5 channel-specific attributes found: attr 0: hysteresis value: 1 attr 1: hysteresis_available value: 0 1 attr 2: label value: position attr 3: raw value: 12578 attr 4: scale value: 0.000095874 anglvel0: (input, index: 1, format: be:S16/16>>0) 3 channel-specific attributes found: attr 0: label value: velocity attr 1: raw value: 5 attr 2: scale value: 0.023968449 timestamp: (input, index: 2, format: le:S64/64>>0) phase0: (input) 1 channel-specific attributes found: attr 0: label value: synthetic reference altvoltage0: (output) 3 channel-specific attributes found: attr 0: frequency value: 10000 attr 1: frequency_available value: [2000 250 20000] attr 2: label value: excitation angl1: (input) 1 channel-specific attributes found: attr 0: label value: tracking error altvoltage1_y: (input) 1 channel-specific attributes found: attr 0: label value: sine altvoltage0: (input) 1 channel-specific attributes found: attr 0: label value: monitor signal altvoltage1_x: (input) 1 channel-specific attributes found: attr 0: label value: cosine 3 device-specific attributes found: attr 0: current_timestamp_clock value: realtime attr 1: fault value: 0x00 attr 2: waiting_for_supplier value: 0 2 buffer-specific attributes found: attr 0: data_available value: 0 attr 1: direction value: in 1 debug attributes found: debug attr 0: direct_reg_access ERROR: Input/output error (5) Current trigger: trigger0(test)
| |