Re: [PATCH v3 00/27] iio: resolver: move ad2s1210 out of staging

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 9/29/23 12:23 PM, David Lechner wrote:
From: David Lechner <david@xxxxxxxxxxxxxx>

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)





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux