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

Message ID 20230929-ad2s1210-mainline-v3-0-fa4364281745@baylibre.com
Headers
Series iio: resolver: move ad2s1210 out of staging |

Message

David Lechner Sept. 29, 2023, 5:23 p.m. UTC
  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
  

Comments

David Lechner Sept. 29, 2023, 10:02 p.m. UTC | #1
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)
  
Jonathan Cameron Sept. 30, 2023, 2:26 p.m. UTC | #2
On Fri, 29 Sep 2023 12:49:42 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On Fri, Sep 29, 2023 at 12:25 PM David Lechner <dlechner@baylibre.com> wrote:
> >
> > From: David Lechner <david@lechnology.com>
> >  
> 
> Ugh, an automated tool picked up my personal email on this for some
> reason. This extra From: line can be dropped from any patches that get
> picked up on this round. I will make sure to double-check next time.

Seems b4 picks up the second, correct From anyway which is handy ;)

Jonathan
  
Jonathan Cameron Sept. 30, 2023, 2:34 p.m. UTC | #3
On Fri, 29 Sep 2023 12:23:06 -0500
David Lechner <dlechner@baylibre.com> wrote:

> From: David Lechner <david@lechnology.com>
> 
> From: David Lechner <dlechner@baylibre.com>
> 
> This adds new DeviceTree bindings for the Analog Devices, Inc. AD2S1210
> resolver-to-digital converter.
> 
> Co-developed-by: Apelete Seketeli <aseketeli@baylibre.com>
> Signed-off-by: Apelete Seketeli <aseketeli@baylibre.com>
> Signed-off-by: David Lechner <dlechner@baylibre.com>

Michael, ideally I'd like your ack on this given it volunteers you as
maintainer. If I don't hear I'm fine with leaving Michael listed
because he's in MAINTAINERS anyway covering these bindings via
a wild card entry:

ANALOG DEVICES INC IIO DRIVERS
M:	Lars-Peter Clausen <lars@metafoo.de>
M:	Michael Hennerich <Michael.Hennerich@analog.com>
...
F:	Documentation/devicetree/bindings/iio/*/adi,*

So any queries should hit Michael anyway.

LGTM but I'll also want the dt binding maintainers input before picking this
up.

Jonathan

> ---
> 
> v3 changes:
> * Expanded top-level description of A0/A1 lines.
> * Added required voltage -supply properties. (I did not pick up Rob's
>   Reviewed-by since I wasn't sure if this was trivial enough.)
> 
> v2 changes:
> * Add Co-developed-by:
> * Remove extraneous quotes on strings
> * Remove extraneous pipe on some multi-line descriptions
> 
>  .../bindings/iio/resolver/adi,ad2s1210.yaml        | 177 +++++++++++++++++++++
>  1 file changed, 177 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/resolver/adi,ad2s1210.yaml b/Documentation/devicetree/bindings/iio/resolver/adi,ad2s1210.yaml
> new file mode 100644
> index 000000000000..8980b3cd8337
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/resolver/adi,ad2s1210.yaml
> @@ -0,0 +1,177 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/resolver/adi,ad2s1210.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD2S1210 Resolver-to-Digital Converter
> +
> +maintainers:
> +  - Michael Hennerich <michael.hennerich@analog.com>
> +
> +description: |
> +  The AD2S1210 is a complete 10-bit to 16-bit resolution tracking
> +  resolver-to-digital converter, integrating an on-board programmable
> +  sinusoidal oscillator that provides sine wave excitation for
> +  resolvers.
> +
> +  The AD2S1210 allows the user to read the angular position or the
> +  angular velocity data directly from the parallel outputs or through
> +  the serial interface.
> +
> +  The mode of operation of the communication channel (parallel or serial) is
> +  selected by the A0 and A1 input pins. In normal mode, data is latched by
> +  toggling the SAMPLE line and can then be read directly. In configuration mode,
> +  data is read or written using a register access scheme (address byte with
> +  read/write flag and data byte).
> +
> +    A1  A0  Result
> +     0   0  Normal mode - position output
> +     0   1  Normal mode - velocity output
> +     1   0  Reserved
> +     1   1  Configuration mode
> +
> +  In normal mode, the resolution of the digital output is selected using
> +  the RES0 and RES1 input pins. In configuration mode, the resolution is
> +  selected by setting the RES0 and RES1 bits in the control register.
> +
> +  RES1  RES0  Resolution (Bits)
> +     0     0  10
> +     0     1  12
> +     1     0  14
> +     1     1  16
> +
> +  Note on SPI connections: The CS line on the AD2S1210 should hard-wired to
> +  logic low and the WR/FSYNC line on the AD2S1210 should be connected to the
> +  SPI CSn output of the SPI controller.
> +
> +  Datasheet:
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/ad2s1210.pdf
> +
> +properties:
> +  compatible:
> +    const: adi,ad2s1210
> +
> +  reg:
> +    maxItems: 1
> +
> +  spi-max-frequency:
> +    maximum: 25000000
> +
> +  spi-cpha: true
> +
> +  avdd-supply:
> +    description:
> +      A 4.75 to 5.25 V regulator that powers the Analog Supply Voltage (AVDD)
> +      pin.
> +
> +  dvdd-supply:
> +    description:
> +      A 4.75 to 5.25 V regulator that powers the Digital Supply Voltage (DVDD)
> +      pin.
> +
> +  vdrive-supply:
> +    description:
> +      A 2.3 to 5.25 V regulator that powers the Logic Power Supply Input
> +      (VDrive) pin.
> +
> +  clocks:
> +    maxItems: 1
> +    description: External oscillator clock (CLKIN).
> +
> +  reset-gpios:
> +    description:
> +      GPIO connected to the /RESET pin. As the line needs to be low for the
> +      reset to be active, it should be configured as GPIO_ACTIVE_LOW.
> +    maxItems: 1
> +
> +  sample-gpios:
> +    description:
> +      GPIO connected to the /SAMPLE pin. As the line needs to be low to trigger
> +      a sample, it should be configured as GPIO_ACTIVE_LOW.
> +    maxItems: 1
> +
> +  mode-gpios:
> +    description:
> +      GPIO lines connected to the A0 and A1 pins. These pins select the data
> +      transfer mode.
> +    minItems: 2
> +    maxItems: 2
> +
> +  resolution-gpios:
> +    description:
> +      GPIO lines connected to the RES0 and RES1 pins. These pins select the
> +      resolution of the digital output. If omitted, it is assumed that the
> +      RES0 and RES1 pins are hard-wired to match the assigned-resolution-bits
> +      property.
> +    minItems: 2
> +    maxItems: 2
> +
> +  fault-gpios:
> +    description:
> +      GPIO lines connected to the LOT and DOS pins. These pins combined indicate
> +      the type of fault present, if any. As these pins a pulled low to indicate
> +      a fault condition, they should be configured as GPIO_ACTIVE_LOW.
> +    minItems: 2
> +    maxItems: 2
> +
> +  adi,fixed-mode:
> +    description:
> +      This is used to indicate the selected mode if A0 and A1 are hard-wired
> +      instead of connected to GPIOS (i.e. mode-gpios is omitted).
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum: [config, velocity, position]
> +
> +  assigned-resolution-bits:
> +    description:
> +      Resolution of the digital output required by the application. This
> +      determines the precision of the angle and/or the maximum speed that can
> +      be measured. If resolution-gpios is omitted, it is assumed that RES0 and
> +      RES1 are hard-wired to match this value.
> +    enum: [10, 12, 14, 16]
> +
> +required:
> +  - compatible
> +  - reg
> +  - spi-cpha
> +  - avdd-supply
> +  - dvdd-supply
> +  - vdrive-supply
> +  - clocks
> +  - sample-gpios
> +  - assigned-resolution-bits
> +
> +oneOf:
> +  - required:
> +      - mode-gpios
> +  - required:
> +      - adi,fixed-mode
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        resolver@0 {
> +            compatible = "adi,ad2s1210";
> +            reg = <0>;
> +            spi-max-frequency = <20000000>;
> +            spi-cpha;
> +            avdd-supply = <&avdd_regulator>;
> +            dvdd-supply = <&dvdd_regulator>;
> +            vdrive-supply = <&vdrive_regulator>;
> +            clocks = <&ext_osc>;
> +            sample-gpios = <&gpio0 90 GPIO_ACTIVE_LOW>;
> +            mode-gpios = <&gpio0 86 0>, <&gpio0 87 0>;
> +            resolution-gpios = <&gpio0 88 0>, <&gpio0 89 0>;
> +            assigned-resolution-bits = <16>;
> +        };
> +    };
>
  
Jonathan Cameron Sept. 30, 2023, 2:52 p.m. UTC | #4
On Fri, 29 Sep 2023 12:23:16 -0500
David Lechner <dlechner@baylibre.com> wrote:

> From: David Lechner <david@lechnology.com>
> 
> From: David Lechner <dlechner@baylibre.com>
> 
> This add an implementation of debugfs_reg_access for the AD2S1210
> driver.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>

I never really care if a driver implements this or not.
However, if you find it useful that's fine, so applied.

Thanks,

Jonathan

> ---
> 
> v3 changes: None
> 
>  drivers/staging/iio/resolver/ad2s1210.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
> index 0663a51d04ad..31415fbb6384 100644
> --- a/drivers/staging/iio/resolver/ad2s1210.c
> +++ b/drivers/staging/iio/resolver/ad2s1210.c
> @@ -614,9 +614,29 @@ static int ad2s1210_initial(struct ad2s1210_state *st)
>  	return ret;
>  }
>  
> +static int ad2s1210_debugfs_reg_access(struct iio_dev *indio_dev,
> +				       unsigned int reg, unsigned int writeval,
> +				       unsigned int *readval)
> +{
> +	struct ad2s1210_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	mutex_lock(&st->lock);
> +
> +	if (readval)
> +		ret = regmap_read(st->regmap, reg, readval);
> +	else
> +		ret = regmap_write(st->regmap, reg, writeval);
> +
> +	mutex_unlock(&st->lock);
> +
> +	return ret;
> +}
> +
>  static const struct iio_info ad2s1210_info = {
>  	.read_raw = ad2s1210_read_raw,
>  	.attrs = &ad2s1210_attribute_group,
> +	.debugfs_reg_access = &ad2s1210_debugfs_reg_access,
>  };
>  
>  static int ad2s1210_setup_clocks(struct ad2s1210_state *st)
>