devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/5] Add driver for LTC2664 and LTC2672
@ 2024-06-19  6:48 Kim Seer Paller
  2024-06-19  6:49 ` [PATCH v4 1/5] iio: ABI: Generalize ABI documentation for DAC Kim Seer Paller
                   ` (4 more replies)
  0 siblings, 5 replies; 24+ messages in thread
From: Kim Seer Paller @ 2024-06-19  6:48 UTC (permalink / raw)
  To: linux-kernel, linux-iio, devicetree
  Cc: Jonathan Cameron, David Lechner, Lars-Peter Clausen,
	Liam Girdwood, Mark Brown, Dimitri Fedrau, Krzysztof Kozlowski,
	Rob Herring, Conor Dooley, Michael Hennerich, Nuno Sá,
	Kim Seer Paller

Generalize the ABI documentation for DAC. The ABI defined for toggle mode
channels:

LTC2664:
  * out_voltageY_toggle_en
  * out_voltageY_raw0
  * out_voltageY_raw1
  * out_voltageY_symbol

LTC2672:
  * out_currentY_toggle_en
  * out_currentY_raw0
  * out_currentY_raw1
  * out_currentY_symbol

Default channels won't have any of the above ABIs. A channel is toggle capable
if the devicetree 'adi,toggle-mode' flag is set.

changes in v4:

ltc2664:
  * Added comments for each field in the ltc2664_chan struct.
  * Changed global_toggle data type to bool and updated vref and rfsadj variables
    to include units.
  * Added 0,0 entry in ltc2672_span_helper to removed the id field from the
    ltc2664_chip_info struct.
  * Used mul_u64_u32_div helper function from linux/math64.h to avoid integer
    overflow during scale calculation.
  * Refactored code to use a single template for channel instead of separate
    channel arrays.
  * Used the devm_regulator_get_enable_read_voltage API for simplifying voltage
    retrieval.

ABI:
  sysfs-bus-iio:
    * Added commit message for the ABI changes.
  sysfs-bus-iio-dac:
    * Updated the description of toggle_en to clarify autonomous toggling.
    * Fixed inconsistent use of spacing and tabs.

Bindings:
  * Updated the description for both bindings to include both 12-bit and 16-bit
    versions.

changes in v3:

ltc2664:
  * Added span sanity check for no match.
  * Initialized the variable 'span' to fix build warning.
  * Added Reported-by and Closes by tag.

ABI:
  * Modified descriptions to make it more generalize.
  * Removed MAINTAINERS file entry.

Bindings:
  * Changed clr-gpios to reset-gpios.
  * Added output range and reset code description for 'adi,manual-span-operation-config'
    property in ltc2664 binding.
  * Removed the $ref for 'adi,output-range-microamp' due to dt-schema warning
    in ltc2672 binding. Added Reported-by and Closes by tag.
  * Modified io-channels description and added maxItems constraint.

changes in v2:

ltc2664:
  * Updated struct ltc2664_chip_info to include device-specific data for scale,
    offset, measurement type, internal vref, manual span support, and rfsadj
    support.
  * Added a read-only extended info attribute powerdown_mode to indicate the
    state that the DAC output enters when the device is powered down.
  * Refactored code for setting the span into separate function and directly
    returning the span.
  * Adjusted memory allocation for st->iio_channels to include null terminator.
  * Spaces have been added after { and before }. Each pair of values is now
    placed on a separate line.

ABI:
  * Generalized the ABI documentation for DAC.
  * Added DAC 42kohm_to_gnd powerdown mode.

Bindings:
  * Created separate bindings for ltc2664 and ltc2672.
  * Added v-pos-supply and v-neg-supply regulator properties.
  * Renamed vref-supply to ref-supply based on the datasheet.
  * Added io-channels property and specifying the pin for multiplexer output.
  * Added vdd0-vdd4 supply properties for ltc2672, although they are not
    currently supported in the driver.
  * Changed clr-gpios description based on the datasheet.
  * Used 4 spaces for example indentation.

Kim Seer Paller (5):
  iio: ABI: Generalize ABI documentation for DAC
  iio: ABI: add DAC 42kohm_to_gnd powerdown mode
  dt-bindings: iio: dac: Add adi,ltc2664.yaml
  dt-bindings: iio: dac: Add adi,ltc2672.yaml
  iio: dac: ltc2664: Add driver for LTC2664 and LTC2672

 Documentation/ABI/testing/sysfs-bus-iio       |   1 +
 Documentation/ABI/testing/sysfs-bus-iio-dac   |  61 ++
 .../ABI/testing/sysfs-bus-iio-dac-ltc2688     |  31 -
 .../bindings/iio/dac/adi,ltc2664.yaml         | 167 ++++
 .../bindings/iio/dac/adi,ltc2672.yaml         | 158 ++++
 MAINTAINERS                                   |  10 +
 drivers/iio/dac/Kconfig                       |  11 +
 drivers/iio/dac/Makefile                      |   1 +
 drivers/iio/dac/ltc2664.c                     | 755 ++++++++++++++++++
 9 files changed, 1164 insertions(+), 31 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac
 create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
 create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml
 create mode 100644 drivers/iio/dac/ltc2664.c


base-commit: 07d4d0bb4a8ddcc463ed599b22f510d5926c2495
-- 
2.34.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v4 1/5] iio: ABI: Generalize ABI documentation for DAC
  2024-06-19  6:48 [PATCH v4 0/5] Add driver for LTC2664 and LTC2672 Kim Seer Paller
@ 2024-06-19  6:49 ` Kim Seer Paller
  2024-06-19  6:49 ` [PATCH v4 2/5] iio: ABI: add DAC 42kohm_to_gnd powerdown mode Kim Seer Paller
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 24+ messages in thread
From: Kim Seer Paller @ 2024-06-19  6:49 UTC (permalink / raw)
  To: linux-kernel, linux-iio, devicetree
  Cc: Jonathan Cameron, David Lechner, Lars-Peter Clausen,
	Liam Girdwood, Mark Brown, Dimitri Fedrau, Krzysztof Kozlowski,
	Rob Herring, Conor Dooley, Michael Hennerich, Nuno Sá,
	Kim Seer Paller

Introduces a more generalized ABI documentation for DAC. Instead of
having separate ABI files for each DAC, we now have a single ABI file
that covers the common sysfs interface for all DAC.

Co-developed-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
---
 Documentation/ABI/testing/sysfs-bus-iio-dac   | 61 +++++++++++++++++++
 .../ABI/testing/sysfs-bus-iio-dac-ltc2688     | 31 ----------
 2 files changed, 61 insertions(+), 31 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac b/Documentation/ABI/testing/sysfs-bus-iio-dac
new file mode 100644
index 000000000000..810eaac5533c
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-dac
@@ -0,0 +1,61 @@
+What:		/sys/bus/iio/devices/iio:deviceX/out_currentY_toggle_en
+KernelVersion:	5.18
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Toggle enable. Write 1 to enable toggle or 0 to disable it. This
+		is useful when one wants to change the DAC output codes. For
+		autonomous toggling, the way it should be done is:
+
+		- disable toggle operation;
+		- change out_currentY_rawN, where N is the integer value of the symbol;
+		- enable toggle operation.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_currentY_rawN
+KernelVersion:	5.18
+Contact:	linux-iio@vger.kernel.org
+Description:
+		This attribute has the same meaning as out_currentY_raw. It is
+		specific to toggle enabled channels and refers to the DAC output
+		code in INPUT_N (_rawN), where N is the integer value of the symbol.
+		The same scale and offset as in out_currentY_raw applies.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_currentY_symbol
+KernelVersion:	5.18
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Performs a SW switch to a predefined output symbol. This attribute
+		is specific to toggle enabled channels and allows switching between
+		multiple predefined symbols. Each symbol corresponds to a different
+		output, denoted as out_currentY_rawN, where N is the integer value
+		of the symbol. Writing an integer value N will select out_currentY_rawN.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_voltageY_toggle_en
+KernelVersion:	5.18
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Toggle enable. Write 1 to enable toggle or 0 to disable it. This
+		is useful when one wants to change the DAC output codes. For
+		autonomous toggling, the way it should be done is:
+
+		- disable toggle operation;
+		- change out_voltageY_rawN, where N is the integer value of the symbol;
+		- enable toggle operation.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_voltageY_rawN
+KernelVersion:	5.18
+Contact:	linux-iio@vger.kernel.org
+Description:
+		This attribute has the same meaning as out_currentY_raw. It is
+		specific to toggle enabled channels and refers to the DAC output
+		code in INPUT_N (_rawN), where N is the integer value of the symbol.
+		The same scale and offset as in out_currentY_raw applies.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_voltageY_symbol
+KernelVersion:	5.18
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Performs a SW switch to a predefined output symbol. This attribute
+		is specific to toggle enabled channels and allows switching between
+		multiple predefined symbols. Each symbol corresponds to a different
+		output, denoted as out_voltageY_rawN, where N is the integer value
+		of the symbol. Writing an integer value N will select out_voltageY_rawN.
diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2688 b/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2688
index 1c35971277ba..ae95a5477382 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2688
+++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2688
@@ -53,34 +53,3 @@ KernelVersion:	5.18
 Contact:	linux-iio@vger.kernel.org
 Description:
 		Returns the available values for the dither phase.
-
-What:		/sys/bus/iio/devices/iio:deviceX/out_voltageY_toggle_en
-KernelVersion:	5.18
-Contact:	linux-iio@vger.kernel.org
-Description:
-		Toggle enable. Write 1 to enable toggle or 0 to disable it. This is
-		useful when one wants to change the DAC output codes. The way it should
-		be done is:
-
-		- disable toggle operation;
-		- change out_voltageY_raw0 and out_voltageY_raw1;
-		- enable toggle operation.
-
-What:		/sys/bus/iio/devices/iio:deviceX/out_voltageY_raw0
-What:		/sys/bus/iio/devices/iio:deviceX/out_voltageY_raw1
-KernelVersion:	5.18
-Contact:	linux-iio@vger.kernel.org
-Description:
-		It has the same meaning as out_voltageY_raw. This attribute is
-		specific to toggle enabled channels and refers to the DAC output
-		code in INPUT_A (_raw0) and INPUT_B (_raw1). The same scale and offset
-		as in out_voltageY_raw applies.
-
-What:		/sys/bus/iio/devices/iio:deviceX/out_voltageY_symbol
-KernelVersion:	5.18
-Contact:	linux-iio@vger.kernel.org
-Description:
-		Performs a SW toggle. This attribute is specific to toggle
-		enabled channels and allows to toggle between out_voltageY_raw0
-		and out_voltageY_raw1 through software. Writing 0 will select
-		out_voltageY_raw0 while 1 selects out_voltageY_raw1.
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v4 2/5] iio: ABI: add DAC 42kohm_to_gnd powerdown mode
  2024-06-19  6:48 [PATCH v4 0/5] Add driver for LTC2664 and LTC2672 Kim Seer Paller
  2024-06-19  6:49 ` [PATCH v4 1/5] iio: ABI: Generalize ABI documentation for DAC Kim Seer Paller
@ 2024-06-19  6:49 ` Kim Seer Paller
  2024-06-19  6:49 ` [PATCH v4 3/5] dt-bindings: iio: dac: Add adi,ltc2664.yaml Kim Seer Paller
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 24+ messages in thread
From: Kim Seer Paller @ 2024-06-19  6:49 UTC (permalink / raw)
  To: linux-kernel, linux-iio, devicetree
  Cc: Jonathan Cameron, David Lechner, Lars-Peter Clausen,
	Liam Girdwood, Mark Brown, Dimitri Fedrau, Krzysztof Kozlowski,
	Rob Herring, Conor Dooley, Michael Hennerich, Nuno Sá,
	Kim Seer Paller

Add a new powerdown mode for DACs with 42kohm resistor to GND.

Co-developed-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
---
 Documentation/ABI/testing/sysfs-bus-iio | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 7cee78ad4108..6ee58f59065b 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -708,6 +708,7 @@ Description:
 		2.5kohm_to_gnd: connected to ground via a 2.5kOhm resistor,
 		6kohm_to_gnd: connected to ground via a 6kOhm resistor,
 		20kohm_to_gnd: connected to ground via a 20kOhm resistor,
+		42kohm_to_gnd: connected to ground via a 42kOhm resistor,
 		90kohm_to_gnd: connected to ground via a 90kOhm resistor,
 		100kohm_to_gnd: connected to ground via an 100kOhm resistor,
 		125kohm_to_gnd: connected to ground via an 125kOhm resistor,
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v4 3/5] dt-bindings: iio: dac: Add adi,ltc2664.yaml
  2024-06-19  6:48 [PATCH v4 0/5] Add driver for LTC2664 and LTC2672 Kim Seer Paller
  2024-06-19  6:49 ` [PATCH v4 1/5] iio: ABI: Generalize ABI documentation for DAC Kim Seer Paller
  2024-06-19  6:49 ` [PATCH v4 2/5] iio: ABI: add DAC 42kohm_to_gnd powerdown mode Kim Seer Paller
@ 2024-06-19  6:49 ` Kim Seer Paller
  2024-06-19 17:56   ` Conor Dooley
  2024-06-19  6:49 ` [PATCH v4 4/5] dt-bindings: iio: dac: Add adi,ltc2672.yaml Kim Seer Paller
  2024-06-19  6:49 ` [PATCH v4 5/5] iio: dac: ltc2664: Add driver for LTC2664 and LTC2672 Kim Seer Paller
  4 siblings, 1 reply; 24+ messages in thread
From: Kim Seer Paller @ 2024-06-19  6:49 UTC (permalink / raw)
  To: linux-kernel, linux-iio, devicetree
  Cc: Jonathan Cameron, David Lechner, Lars-Peter Clausen,
	Liam Girdwood, Mark Brown, Dimitri Fedrau, Krzysztof Kozlowski,
	Rob Herring, Conor Dooley, Michael Hennerich, Nuno Sá,
	Kim Seer Paller

Add documentation for ltc2664.

Co-developed-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
---
 .../bindings/iio/dac/adi,ltc2664.yaml         | 167 ++++++++++++++++++
 MAINTAINERS                                   |   8 +
 2 files changed, 175 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml

diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
new file mode 100644
index 000000000000..be37700e3b1f
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
@@ -0,0 +1,167 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/dac/adi,ltc2664.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices LTC2664 DAC
+
+maintainers:
+  - Michael Hennerich <michael.hennerich@analog.com>
+  - Kim Seer Paller <kimseer.paller@analog.com>
+
+description: |
+  Analog Devices LTC2664 4 channel, 12-/16-Bit, +-10V DAC
+  https://www.analog.com/media/en/technical-documentation/data-sheets/2664fa.pdf
+
+properties:
+  compatible:
+    enum:
+      - adi,ltc2664
+
+  reg:
+    maxItems: 1
+
+  spi-max-frequency:
+    maximum: 50000000
+
+  vcc-supply:
+    description: Analog Supply Voltage Input.
+
+  v-pos-supply:
+    description: Positive Supply Voltage Input.
+
+  v-neg-supply:
+    description: Negative Supply Voltage Input.
+
+  iovcc-supply:
+    description: Digital Input/Output Supply Voltage.
+
+  ref-supply:
+    description:
+      Reference Input/Output. The voltage at the REF pin sets the full-scale
+      range of all channels. If not provided the internal reference is used and
+      also provided on the VREF pin.
+
+  reset-gpios:
+    description:
+      Active-low Asynchronous Clear Input. A logic low at this level-triggered
+      input clears the part to the reset code and range determined by the
+      hardwired option chosen using the MSPAN pins. The control registers are
+      cleared to zero.
+    maxItems: 1
+
+  adi,manual-span-operation-config:
+    description:
+      This property must mimic the MSPAN pin configurations. By tying the MSPAN
+      pins (MSP2, MSP1 and MSP0) to GND and/or VCC, any output range can be
+      hardware-configured with different mid-scale or zero-scale reset options.
+      The hardware configuration is latched during power on reset for proper
+      operation.
+        0 - MPS2=GND, MPS1=GND, MSP0=GND (+-10V, reset to 0V)
+        1 - MPS2=GND, MPS1=GND, MSP0=VCC (+-5V, reset to 0V)
+        2 - MPS2=GND, MPS1=VCC, MSP0=GND (+-2.5V, reset to 0V)
+        3 - MPS2=GND, MPS1=VCC, MSP0=VCC (0V to 10, reset to 0V)
+        4 - MPS2=VCC, MPS1=GND, MSP0=GND (0V to 10V, reset to 5V)
+        5 - MPS2=VCC, MPS1=GND, MSP0=VCC (0V to 5V, reset to 0V)
+        6 - MPS2=VCC, MPS1=VCC, MSP0=GND (0V to 5V, reset to 2.5V)
+        7 - MPS2=VCC, MPS1=VCC, MSP0=VCC (0V to 5V, reset to 0V, enables SoftSpan)
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1, 2, 3, 4, 5, 6, 7]
+    default: 7
+
+  io-channels:
+    description:
+      ADC channel to monitor voltages and temperature at the MUXOUT pin.
+    maxItems: 1
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+patternProperties:
+  "^channel@[0-3]$":
+    type: object
+    additionalProperties: false
+
+    properties:
+      reg:
+        description: The channel number representing the DAC output channel.
+        maximum: 3
+
+      adi,toggle-mode:
+        description:
+          Set the channel as a toggle enabled channel. Toggle operation enables
+          fast switching of a DAC output between two different DAC codes without
+          any SPI transaction.
+        type: boolean
+
+      adi,output-range-microvolt:
+        description: Specify the channel output full scale range.
+        oneOf:
+          - items:
+              - const: 0
+              - enum: [5000000, 10000000]
+          - items:
+              - const: -5000000
+              - const: 5000000
+          - items:
+              - const: -10000000
+              - const: 10000000
+          - items:
+              - const: -2500000
+              - const: 2500000
+
+    required:
+      - reg
+      - adi,output-range-microvolt
+
+required:
+  - compatible
+  - reg
+  - spi-max-frequency
+  - vcc-supply
+  - iovcc-supply
+  - v-pos-supply
+  - v-neg-supply
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+additionalProperties: false
+
+examples:
+  - |
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        dac@0 {
+            compatible = "adi,ltc2664";
+            reg = <0>;
+            spi-max-frequency = <10000000>;
+
+            vcc-supply = <&vcc>;
+            iovcc-supply = <&vcc>;
+            ref-supply = <&vref>;
+            v-pos-supply = <&vpos>;
+            v-neg-supply = <&vneg>;
+
+            io-channels = <&adc 0>;
+
+            #address-cells = <1>;
+            #size-cells = <0>;
+            channel@0 {
+                    reg = <0>;
+                    adi,toggle-mode;
+                    adi,output-range-microvolt = <(-10000000) 10000000>;
+            };
+
+            channel@1 {
+                    reg = <1>;
+                    adi,output-range-microvolt = <0 10000000>;
+            };
+        };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index be590c462d91..849800d9cbf7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13074,6 +13074,14 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/iio/dac/lltc,ltc1660.yaml
 F:	drivers/iio/dac/ltc1660.c
 
+LTC2664 IIO DAC DRIVER
+M:	Michael Hennerich <michael.hennerich@analog.com>
+M:	Kim Seer Paller <kimseer.paller@analog.com>
+L:	linux-iio@vger.kernel.org
+S:	Supported
+W:	https://ez.analog.com/linux-software-drivers
+F:	Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
+
 LTC2688 IIO DAC DRIVER
 M:	Nuno Sá <nuno.sa@analog.com>
 L:	linux-iio@vger.kernel.org
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v4 4/5] dt-bindings: iio: dac: Add adi,ltc2672.yaml
  2024-06-19  6:48 [PATCH v4 0/5] Add driver for LTC2664 and LTC2672 Kim Seer Paller
                   ` (2 preceding siblings ...)
  2024-06-19  6:49 ` [PATCH v4 3/5] dt-bindings: iio: dac: Add adi,ltc2664.yaml Kim Seer Paller
@ 2024-06-19  6:49 ` Kim Seer Paller
  2024-06-19 17:57   ` Conor Dooley
  2024-06-19  6:49 ` [PATCH v4 5/5] iio: dac: ltc2664: Add driver for LTC2664 and LTC2672 Kim Seer Paller
  4 siblings, 1 reply; 24+ messages in thread
From: Kim Seer Paller @ 2024-06-19  6:49 UTC (permalink / raw)
  To: linux-kernel, linux-iio, devicetree
  Cc: Jonathan Cameron, David Lechner, Lars-Peter Clausen,
	Liam Girdwood, Mark Brown, Dimitri Fedrau, Krzysztof Kozlowski,
	Rob Herring, Conor Dooley, Michael Hennerich, Nuno Sá,
	Kim Seer Paller

Add documentation for ltc2672.

Co-developed-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
---
 .../bindings/iio/dac/adi,ltc2672.yaml         | 158 ++++++++++++++++++
 MAINTAINERS                                   |   1 +
 2 files changed, 159 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml

diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml
new file mode 100644
index 000000000000..0ccf53fb22cc
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml
@@ -0,0 +1,158 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/dac/adi,ltc2672.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices LTC2672 DAC
+
+maintainers:
+  - Michael Hennerich <michael.hennerich@analog.com>
+  - Kim Seer Paller <kimseer.paller@analog.com>
+
+description: |
+  Analog Devices LTC2672 5 channel, 12-/16-Bit, 300mA DAC
+  https://www.analog.com/media/en/technical-documentation/data-sheets/ltc2672.pdf
+
+properties:
+  compatible:
+    enum:
+      - adi,ltc2672
+
+  reg:
+    maxItems: 1
+
+  spi-max-frequency:
+    maximum: 50000000
+
+  vcc-supply:
+    description: Analog Supply Voltage Input.
+
+  v-neg-supply:
+    description: Negative Supply Voltage Input.
+
+  vdd0-supply:
+    description: Positive Supply Voltage Input for DAC OUT0.
+
+  vdd1-supply:
+    description: Positive Supply Voltage Input for DAC OUT1.
+
+  vdd2-supply:
+    description: Positive Supply Voltage Input for DAC OUT2.
+
+  vdd3-supply:
+    description: Positive Supply Voltage Input for DAC OUT3.
+
+  vdd4-supply:
+    description: Positive Supply Voltage Input for DAC OUT4.
+
+  iovcc-supply:
+    description: Digital Input/Output Supply Voltage.
+
+  ref-supply:
+    description:
+      Reference Input/Output. The voltage at the REF pin sets the full-scale
+      range of all channels. If not provided the internal reference is used and
+      also provided on the VREF pin.
+
+  reset-gpios:
+    description:
+      Active Low Asynchronous Clear Input. A logic low at this level triggered
+      input clears the device to the default reset code and output range, which
+      is zero-scale with the outputs off. The control registers are cleared to
+      zero.
+    maxItems: 1
+
+  adi,rfsadj-ohms:
+    description:
+      If FSADJ is tied to VCC, an internal RFSADJ (20 kΩ) is selected, which
+      results in nominal output ranges. When an external resistor of 19 kΩ to
+      41 kΩ can be used instead by connecting the resistor between FSADJ and GND
+      it controls the scaling of the ranges, and the internal resistor is
+      automatically disconnected.
+    minimum: 19000
+    maximum: 41000
+    default: 20000
+
+  io-channels:
+    description:
+      ADC channel to monitor voltages and currents at the MUX pin.
+    maxItems: 1
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+patternProperties:
+  "^channel@[0-4]$":
+    type: object
+    additionalProperties: false
+
+    properties:
+      reg:
+        description: The channel number representing the DAC output channel.
+        maximum: 4
+
+      adi,toggle-mode:
+        description:
+          Set the channel as a toggle enabled channel. Toggle operation enables
+          fast switching of a DAC output between two different DAC codes without
+          any SPI transaction.
+        type: boolean
+
+      adi,output-range-microamp:
+        description: Specify the channel output full scale range.
+        enum: [3125000, 6250000, 12500000, 25000000, 50000000, 100000000,
+               200000000, 300000000]
+
+    required:
+      - reg
+      - adi,output-range-microamp
+
+required:
+  - compatible
+  - reg
+  - spi-max-frequency
+  - vcc-supply
+  - iovcc-supply
+  - v-neg-supply
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+additionalProperties: false
+
+examples:
+  - |
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        dac@0 {
+            compatible = "adi,ltc2672";
+            reg = <0>;
+            spi-max-frequency = <10000000>;
+
+            vcc-supply = <&vcc>;
+            iovcc-supply = <&vcc>;
+            ref-supply = <&vref>;
+            v-neg-supply = <&vneg>;
+
+            io-channels = <&adc 0>;
+
+            #address-cells = <1>;
+            #size-cells = <0>;
+            channel@0 {
+                    reg = <0>;
+                    adi,toggle-mode;
+                    adi,output-range-microamp = <3125000>;
+            };
+
+            channel@1 {
+                    reg = <1>;
+                    adi,output-range-microamp = <6250000>;
+            };
+        };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 849800d9cbf7..f4a5b5bc8ccc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13081,6 +13081,7 @@ L:	linux-iio@vger.kernel.org
 S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
+F:	Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml
 
 LTC2688 IIO DAC DRIVER
 M:	Nuno Sá <nuno.sa@analog.com>
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v4 5/5] iio: dac: ltc2664: Add driver for LTC2664 and LTC2672
  2024-06-19  6:48 [PATCH v4 0/5] Add driver for LTC2664 and LTC2672 Kim Seer Paller
                   ` (3 preceding siblings ...)
  2024-06-19  6:49 ` [PATCH v4 4/5] dt-bindings: iio: dac: Add adi,ltc2672.yaml Kim Seer Paller
@ 2024-06-19  6:49 ` Kim Seer Paller
  2024-06-20  0:00   ` Paller, Kim Seer
                     ` (2 more replies)
  4 siblings, 3 replies; 24+ messages in thread
From: Kim Seer Paller @ 2024-06-19  6:49 UTC (permalink / raw)
  To: linux-kernel, linux-iio, devicetree
  Cc: Jonathan Cameron, David Lechner, Lars-Peter Clausen,
	Liam Girdwood, Mark Brown, Dimitri Fedrau, Krzysztof Kozlowski,
	Rob Herring, Conor Dooley, Michael Hennerich, Nuno Sá,
	Kim Seer Paller

LTC2664 4 channel, 12-/16-Bit Voltage Output SoftSpan DAC
LTC2672 5 channel, 12-/16-Bit Current Output Softspan DAC

Co-developed-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
---
 MAINTAINERS               |   1 +
 drivers/iio/dac/Kconfig   |  11 +
 drivers/iio/dac/Makefile  |   1 +
 drivers/iio/dac/ltc2664.c | 755 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 768 insertions(+)
 create mode 100644 drivers/iio/dac/ltc2664.c

diff --git a/MAINTAINERS b/MAINTAINERS
index f4a5b5bc8ccc..7a02d9a196fb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13082,6 +13082,7 @@ S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
 F:	Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml
+F:	drivers/iio/dac/ltc2664.c
 
 LTC2688 IIO DAC DRIVER
 M:	Nuno Sá <nuno.sa@analog.com>
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index fb48dddbcc20..3a7691db3998 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -371,6 +371,17 @@ config LTC2632
 	  To compile this driver as a module, choose M here: the
 	  module will be called ltc2632.
 
+config LTC2664
+	tristate "Analog Devices LTC2664 and LTC2672 DAC SPI driver"
+	depends on SPI
+	select REGMAP
+	help
+	  Say yes here to build support for Analog Devices
+	  LTC2664 and LTC2672 converters (DAC).
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ltc2664.
+
 config M62332
 	tristate "Mitsubishi M62332 DAC driver"
 	depends on I2C
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 8432a81a19dc..2cf148f16306 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -37,6 +37,7 @@ obj-$(CONFIG_DS4424) += ds4424.o
 obj-$(CONFIG_LPC18XX_DAC) += lpc18xx_dac.o
 obj-$(CONFIG_LTC1660) += ltc1660.o
 obj-$(CONFIG_LTC2632) += ltc2632.o
+obj-$(CONFIG_LTC2664) += ltc2664.o
 obj-$(CONFIG_LTC2688) += ltc2688.o
 obj-$(CONFIG_M62332) += m62332.o
 obj-$(CONFIG_MAX517) += max517.o
diff --git a/drivers/iio/dac/ltc2664.c b/drivers/iio/dac/ltc2664.c
new file mode 100644
index 000000000000..9b73b9c6a7a7
--- /dev/null
+++ b/drivers/iio/dac/ltc2664.c
@@ -0,0 +1,755 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * LTC2664 4 channel, 12-/16-Bit Voltage Output SoftSpan DAC driver
+ * LTC2672 5 channel, 12-/16-Bit Current Output Softspan DAC driver
+ *
+ * Copyright 2024 Analog Devices Inc.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/cleanup.h>
+#include <linux/device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/iio/iio.h>
+#include <linux/kernel.h>
+#include <linux/math64.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/mutex.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#define LTC2664_CMD_WRITE_N(n)		(0x00 + (n))
+#define LTC2664_CMD_UPDATE_N(n)		(0x10 + (n))
+#define LTC2664_CMD_WRITE_N_UPDATE_ALL	0x20
+#define LTC2664_CMD_WRITE_N_UPDATE_N(n)	(0x30 + (n))
+#define LTC2664_CMD_POWER_DOWN_N(n)	(0x40 + (n))
+#define LTC2664_CMD_POWER_DOWN_ALL	0x50
+#define LTC2664_CMD_SPAN_N(n)		(0x60 + (n))
+#define LTC2664_CMD_CONFIG		0x70
+#define LTC2664_CMD_MUX			0xB0
+#define LTC2664_CMD_TOGGLE_SEL		0xC0
+#define LTC2664_CMD_GLOBAL_TOGGLE	0xD0
+#define LTC2664_CMD_NO_OPERATION	0xF0
+#define LTC2664_REF_DISABLE		0x0001
+#define LTC2664_MSPAN_SOFTSPAN		7
+
+#define LTC2672_MAX_CHANNEL		5
+#define LTC2672_MAX_SPAN		7
+#define LTC2672_SCALE_MULTIPLIER(n)	(50 * BIT(n))
+
+enum ltc2664_ids {
+	LTC2664,
+	LTC2672,
+};
+
+enum {
+	LTC2664_SPAN_RANGE_0V_5V,
+	LTC2664_SPAN_RANGE_0V_10V,
+	LTC2664_SPAN_RANGE_M5V_5V,
+	LTC2664_SPAN_RANGE_M10V_10V,
+	LTC2664_SPAN_RANGE_M2V5_2V5,
+};
+
+enum {
+	LTC2664_INPUT_A,
+	LTC2664_INPUT_B,
+	LTC2664_INPUT_B_AVAIL,
+	LTC2664_POWERDOWN,
+	LTC2664_POWERDOWN_MODE,
+	LTC2664_TOGGLE_EN,
+	LTC2664_GLOBAL_TOGGLE,
+};
+
+static const u16 ltc2664_mspan_lut[8][2] = {
+	{ LTC2664_SPAN_RANGE_M10V_10V, 32768 }, /* MPS2=0, MPS1=0, MSP0=0 (0)*/
+	{ LTC2664_SPAN_RANGE_M5V_5V, 32768 }, /* MPS2=0, MPS1=0, MSP0=1 (1)*/
+	{ LTC2664_SPAN_RANGE_M2V5_2V5, 32768 }, /* MPS2=0, MPS1=1, MSP0=0 (2)*/
+	{ LTC2664_SPAN_RANGE_0V_10V, 0 }, /* MPS2=0, MPS1=1, MSP0=1 (3)*/
+	{ LTC2664_SPAN_RANGE_0V_10V, 32768 }, /* MPS2=1, MPS1=0, MSP0=0 (4)*/
+	{ LTC2664_SPAN_RANGE_0V_5V, 0 }, /* MPS2=1, MPS1=0, MSP0=1 (5)*/
+	{ LTC2664_SPAN_RANGE_0V_5V, 32768 }, /* MPS2=1, MPS1=1, MSP0=0 (6)*/
+	{ LTC2664_SPAN_RANGE_0V_5V, 0 } /* MPS2=1, MPS1=1, MSP0=1 (7)*/
+};
+
+struct ltc2664_state;
+
+struct ltc2664_chip_info {
+	enum ltc2664_ids id;
+	const char *name;
+	int (*scale_get)(const struct ltc2664_state *st, int c);
+	int (*offset_get)(const struct ltc2664_state *st, int c);
+	int measurement_type;
+	unsigned int num_channels;
+	const struct iio_chan_spec *iio_chan;
+	const int (*span_helper)[2];
+	unsigned int num_span;
+	unsigned int internal_vref_mv;
+	bool manual_span_support;
+	bool rfsadj_support;
+};
+
+struct ltc2664_chan {
+	/* indicates if the channel should be toggled */
+	bool toggle_chan;
+	/* indicates if the channel is in powered down state */
+	bool powerdown;
+	/* span code of the channel */
+	u8 span;
+	/* raw data of the current state of the chip registers (A/B) */
+	u16 raw[2];
+};
+
+struct ltc2664_state {
+	struct spi_device *spi;
+	struct regmap *regmap;
+	struct ltc2664_chan channels[LTC2672_MAX_CHANNEL];
+	/* lock to protect against multiple access to the device and shared data */
+	struct mutex lock;
+	const struct ltc2664_chip_info *chip_info;
+	struct iio_chan_spec *iio_channels;
+	int vref_mv;
+	u32 rfsadj_ohms;
+	u32 toggle_sel;
+	bool global_toggle;
+};
+
+static const int ltc2664_span_helper[][2] = {
+	{ 0, 5000 },
+	{ 0, 10000 },
+	{ -5000, 5000 },
+	{ -10000, 10000 },
+	{ -2500, 2500 },
+};
+
+static const int ltc2672_span_helper[][2] = {
+	{ 0, 0 },
+	{ 0, 3125 },
+	{ 0, 6250 },
+	{ 0, 12500 },
+	{ 0, 25000 },
+	{ 0, 50000 },
+	{ 0, 100000 },
+	{ 0, 200000 },
+	{ 0, 300000 },
+};
+
+static int ltc2664_scale_get(const struct ltc2664_state *st, int c)
+{
+	const struct ltc2664_chan *chan = &st->channels[c];
+	const int (*span_helper)[2] = st->chip_info->span_helper;
+	int span, fs;
+
+	span = chan->span;
+	if (span < 0)
+		return span;
+
+	fs = span_helper[span][1] - span_helper[span][0];
+
+	return fs * st->vref_mv / 2500;
+}
+
+static int ltc2672_scale_get(const struct ltc2664_state *st, int c)
+{
+	const struct ltc2664_chan *chan = &st->channels[c];
+	int span, fs;
+
+	span = chan->span - 1;
+	if (span < 0)
+		return span;
+
+	fs = 1000 * st->vref_mv;
+
+	if (span == LTC2672_MAX_SPAN)
+		return mul_u64_u32_div(4800, fs, st->rfsadj_ohms);
+
+	return mul_u64_u32_div(LTC2672_SCALE_MULTIPLIER(span), fs, st->rfsadj_ohms);
+}
+
+static int ltc2664_offset_get(const struct ltc2664_state *st, int c)
+{
+	const struct ltc2664_chan *chan = &st->channels[c];
+	int span;
+
+	span = chan->span;
+	if (span < 0)
+		return span;
+
+	if (st->chip_info->span_helper[span][0] < 0)
+		return -32768;
+
+	return 0;
+}
+
+static int ltc2664_dac_code_write(struct ltc2664_state *st, u32 chan, u32 input,
+				  u16 code)
+{
+	struct ltc2664_chan *c = &st->channels[chan];
+	int ret, reg;
+
+	guard(mutex)(&st->lock);
+	/* select the correct input register to write to */
+	if (c->toggle_chan) {
+		ret = regmap_write(st->regmap, LTC2664_CMD_TOGGLE_SEL,
+				   input << chan);
+		if (ret)
+			return ret;
+	}
+	/*
+	 * If in toggle mode the dac should be updated by an
+	 * external signal (or sw toggle) and not here.
+	 */
+	if (st->toggle_sel & BIT(chan))
+		reg = LTC2664_CMD_WRITE_N(chan);
+	else
+		reg = LTC2664_CMD_WRITE_N_UPDATE_N(chan);
+
+	ret = regmap_write(st->regmap, reg, code);
+	if (ret)
+		return ret;
+
+	c->raw[input] = code;
+
+	if (c->toggle_chan) {
+		ret = regmap_write(st->regmap, LTC2664_CMD_TOGGLE_SEL,
+				   st->toggle_sel);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int ltc2664_dac_code_read(struct ltc2664_state *st, u32 chan, u32 input,
+				 u32 *code)
+{
+	guard(mutex)(&st->lock);
+	*code = st->channels[chan].raw[input];
+
+	return 0;
+}
+
+static const int ltc2664_raw_range[] = { 0, 1, U16_MAX };
+
+static int ltc2664_read_avail(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      const int **vals, int *type, int *length,
+			      long info)
+{
+	switch (info) {
+	case IIO_CHAN_INFO_RAW:
+		*vals = ltc2664_raw_range;
+		*type = IIO_VAL_INT;
+
+		return IIO_AVAIL_RANGE;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ltc2664_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan, int *val,
+			    int *val2, long info)
+{
+	struct ltc2664_state *st = iio_priv(indio_dev);
+	int ret;
+
+	switch (info) {
+	case IIO_CHAN_INFO_RAW:
+		ret = ltc2664_dac_code_read(st, chan->channel,
+					    LTC2664_INPUT_A, val);
+		if (ret)
+			return ret;
+
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_OFFSET:
+		*val = st->chip_info->offset_get(st, chan->channel);
+
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		*val = st->chip_info->scale_get(st, chan->channel);
+
+		*val2 = 16;
+		return IIO_VAL_FRACTIONAL_LOG2;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ltc2664_write_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan, int val,
+			     int val2, long info)
+{
+	struct ltc2664_state *st = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_CHAN_INFO_RAW:
+		if (val > U16_MAX || val < 0)
+			return -EINVAL;
+
+		return ltc2664_dac_code_write(st, chan->channel,
+					      LTC2664_INPUT_A, val);
+	default:
+		return -EINVAL;
+	}
+}
+
+static ssize_t ltc2664_reg_bool_get(struct iio_dev *indio_dev,
+				    uintptr_t private,
+				    const struct iio_chan_spec *chan,
+				    char *buf)
+{
+	struct ltc2664_state *st = iio_priv(indio_dev);
+	u32 val;
+
+	guard(mutex)(&st->lock);
+	switch (private) {
+	case LTC2664_POWERDOWN:
+		val = st->channels[chan->channel].powerdown;
+
+		return sysfs_emit(buf, "%u\n", val);
+	case LTC2664_POWERDOWN_MODE:
+		return sysfs_emit(buf, "42kohm_to_gnd\n");
+	case LTC2664_TOGGLE_EN:
+		val = !!(st->toggle_sel & BIT(chan->channel));
+
+		return sysfs_emit(buf, "%u\n", val);
+	case LTC2664_GLOBAL_TOGGLE:
+		val = st->global_toggle;
+
+		return sysfs_emit(buf, "%u\n", val);
+	default:
+		return -EINVAL;
+	}
+}
+
+static ssize_t ltc2664_reg_bool_set(struct iio_dev *indio_dev,
+				    uintptr_t private,
+				    const struct iio_chan_spec *chan,
+				    const char *buf, size_t len)
+{
+	struct ltc2664_state *st = iio_priv(indio_dev);
+	int ret;
+	bool en;
+
+	ret = kstrtobool(buf, &en);
+	if (ret)
+		return ret;
+
+	guard(mutex)(&st->lock);
+	switch (private) {
+	case LTC2664_POWERDOWN:
+		ret = regmap_write(st->regmap,
+				   en ? LTC2664_CMD_POWER_DOWN_N(chan->channel) :
+				   LTC2664_CMD_UPDATE_N(chan->channel), en);
+		if (ret)
+			return ret;
+
+		st->channels[chan->channel].powerdown = en;
+
+		return len;
+	case LTC2664_TOGGLE_EN:
+		if (en)
+			st->toggle_sel |= BIT(chan->channel);
+		else
+			st->toggle_sel &= ~BIT(chan->channel);
+
+		ret = regmap_write(st->regmap, LTC2664_CMD_TOGGLE_SEL,
+				   st->toggle_sel);
+		if (ret)
+			return ret;
+
+		return len;
+	case LTC2664_GLOBAL_TOGGLE:
+		ret = regmap_write(st->regmap, LTC2664_CMD_GLOBAL_TOGGLE, en);
+		if (ret)
+			return ret;
+
+		st->global_toggle = en;
+
+		return len;
+	default:
+		return -EINVAL;
+	}
+}
+
+static ssize_t ltc2664_dac_input_read(struct iio_dev *indio_dev,
+				      uintptr_t private,
+				      const struct iio_chan_spec *chan,
+				      char *buf)
+{
+	struct ltc2664_state *st = iio_priv(indio_dev);
+	int ret;
+	u32 val;
+
+	if (private == LTC2664_INPUT_B_AVAIL)
+		return sysfs_emit(buf, "[%u %u %u]\n", ltc2664_raw_range[0],
+				  ltc2664_raw_range[1],
+				  ltc2664_raw_range[2] / 4);
+
+	ret = ltc2664_dac_code_read(st, chan->channel, private, &val);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%u\n", val);
+}
+
+static ssize_t ltc2664_dac_input_write(struct iio_dev *indio_dev,
+				       uintptr_t private,
+				       const struct iio_chan_spec *chan,
+				       const char *buf, size_t len)
+{
+	struct ltc2664_state *st = iio_priv(indio_dev);
+	int ret;
+	u16 val;
+
+	if (private == LTC2664_INPUT_B_AVAIL)
+		return -EINVAL;
+
+	ret = kstrtou16(buf, 10, &val);
+	if (ret)
+		return ret;
+
+	ret = ltc2664_dac_code_write(st, chan->channel, private, val);
+	if (ret)
+		return ret;
+
+	return len;
+}
+
+static int ltc2664_reg_access(struct iio_dev *indio_dev,
+			      unsigned int reg,
+			      unsigned int writeval,
+			      unsigned int *readval)
+{
+	struct ltc2664_state *st = iio_priv(indio_dev);
+
+	if (readval)
+		return -EOPNOTSUPP;
+
+	return regmap_write(st->regmap, reg, writeval);
+}
+
+#define LTC2664_CHAN_EXT_INFO(_name, _what, _shared, _read, _write) {	\
+	.name = _name,							\
+	.read = (_read),						\
+	.write = (_write),						\
+	.private = (_what),						\
+	.shared = (_shared),						\
+}
+
+/*
+ * For toggle mode we only expose the symbol attr (sw_toggle) in case a TGPx is
+ * not provided in dts.
+ */
+static const struct iio_chan_spec_ext_info ltc2664_toggle_sym_ext_info[] = {
+	LTC2664_CHAN_EXT_INFO("raw0", LTC2664_INPUT_A, IIO_SEPARATE,
+			      ltc2664_dac_input_read, ltc2664_dac_input_write),
+	LTC2664_CHAN_EXT_INFO("raw1", LTC2664_INPUT_B, IIO_SEPARATE,
+			      ltc2664_dac_input_read, ltc2664_dac_input_write),
+	LTC2664_CHAN_EXT_INFO("powerdown", LTC2664_POWERDOWN, IIO_SEPARATE,
+			      ltc2664_reg_bool_get, ltc2664_reg_bool_set),
+	LTC2664_CHAN_EXT_INFO("powerdown_mode", LTC2664_POWERDOWN_MODE,
+			      IIO_SEPARATE, ltc2664_reg_bool_get, NULL),
+	LTC2664_CHAN_EXT_INFO("symbol", LTC2664_GLOBAL_TOGGLE, IIO_SEPARATE,
+			      ltc2664_reg_bool_get, ltc2664_reg_bool_set),
+	LTC2664_CHAN_EXT_INFO("toggle_en", LTC2664_TOGGLE_EN,
+			      IIO_SEPARATE, ltc2664_reg_bool_get,
+			      ltc2664_reg_bool_set),
+	{ }
+};
+
+static const struct iio_chan_spec_ext_info ltc2664_ext_info[] = {
+	LTC2664_CHAN_EXT_INFO("powerdown", LTC2664_POWERDOWN, IIO_SEPARATE,
+			      ltc2664_reg_bool_get, ltc2664_reg_bool_set),
+	LTC2664_CHAN_EXT_INFO("powerdown_mode", LTC2664_POWERDOWN_MODE,
+			      IIO_SEPARATE, ltc2664_reg_bool_get, NULL),
+	{ }
+};
+
+static const struct iio_chan_spec ltc2664_channel_template = {
+	.indexed = 1,
+	.output = 1,
+	.info_mask_separate = BIT(IIO_CHAN_INFO_SCALE) |
+			      BIT(IIO_CHAN_INFO_OFFSET) |
+			      BIT(IIO_CHAN_INFO_RAW),
+	.info_mask_separate_available = BIT(IIO_CHAN_INFO_RAW),
+	.ext_info = ltc2664_ext_info,
+};
+
+static const struct ltc2664_chip_info ltc2664_chip = {
+	.name = "ltc2664",
+	.scale_get = ltc2664_scale_get,
+	.offset_get = ltc2664_offset_get,
+	.measurement_type = IIO_VOLTAGE,
+	.num_channels = 4,
+	.span_helper = ltc2664_span_helper,
+	.num_span = ARRAY_SIZE(ltc2664_span_helper),
+	.internal_vref_mv = 2500,
+	.manual_span_support = true,
+	.rfsadj_support = false,
+};
+
+static const struct ltc2664_chip_info ltc2672_chip = {
+	.name = "ltc2672",
+	.scale_get = ltc2672_scale_get,
+	.offset_get = ltc2664_offset_get,
+	.measurement_type = IIO_CURRENT,
+	.num_channels = 5,
+	.span_helper = ltc2672_span_helper,
+	.num_span = ARRAY_SIZE(ltc2672_span_helper),
+	.internal_vref_mv = 1250,
+	.manual_span_support = false,
+	.rfsadj_support = true,
+};
+
+static int ltc2664_set_span(const struct ltc2664_state *st, int min, int max,
+			    int chan)
+{
+	const struct ltc2664_chip_info *chip_info = st->chip_info;
+	const int (*span_helper)[2] = chip_info->span_helper;
+	int span, ret;
+
+	for (span = 0; span < chip_info->num_span; span++) {
+		if (min == span_helper[span][0] && max == span_helper[span][1])
+			break;
+	}
+
+	if (span == chip_info->num_span)
+		return -EINVAL;
+
+	ret = regmap_write(st->regmap, LTC2664_CMD_SPAN_N(chan), span);
+	if (ret)
+		return ret;
+
+	return span;
+}
+
+static int ltc2664_channel_config(struct ltc2664_state *st)
+{
+	const struct ltc2664_chip_info *chip_info = st->chip_info;
+	struct device *dev = &st->spi->dev;
+	u32 reg, tmp[2], mspan;
+	int ret, span = 0;
+
+	mspan = LTC2664_MSPAN_SOFTSPAN;
+	ret = device_property_read_u32(dev, "adi,manual-span-operation-config",
+				       &mspan);
+	if (!ret) {
+		if (!chip_info->manual_span_support)
+			return dev_err_probe(dev, -EINVAL,
+			       "adi,manual-span-operation-config not supported\n");
+
+		if (mspan > ARRAY_SIZE(ltc2664_mspan_lut))
+			return dev_err_probe(dev, -EINVAL,
+			       "adi,manual-span-operation-config not in range\n");
+	}
+
+	st->rfsadj_ohms = 20000;
+	ret = device_property_read_u32(dev, "adi,rfsadj-ohms", &st->rfsadj_ohms);
+	if (!ret) {
+		if (!chip_info->rfsadj_support)
+			return dev_err_probe(dev, -EINVAL,
+					     "adi,rfsadj-ohms not supported\n");
+
+		if (st->rfsadj_ohms < 19000 || st->rfsadj_ohms > 41000)
+			return dev_err_probe(dev, -EINVAL,
+					     "adi,rfsadj-ohms not in range\n");
+	}
+
+	device_for_each_child_node_scoped(dev, child) {
+		struct ltc2664_chan *chan;
+
+		ret = fwnode_property_read_u32(child, "reg", &reg);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "Failed to get reg property\n");
+
+		if (reg >= chip_info->num_channels)
+			return dev_err_probe(dev, -EINVAL,
+					     "reg bigger than: %d\n",
+					     chip_info->num_channels);
+
+		chan = &st->channels[reg];
+
+		if (fwnode_property_read_bool(child, "adi,toggle-mode")) {
+			chan->toggle_chan = true;
+			/* assume sw toggle ABI */
+			st->iio_channels[reg].ext_info = ltc2664_toggle_sym_ext_info;
+
+			/*
+			 * Clear IIO_CHAN_INFO_RAW bit as toggle channels expose
+			 * out_voltage/current_raw{0|1} files.
+			 */
+			__clear_bit(IIO_CHAN_INFO_RAW,
+				    &st->iio_channels[reg].info_mask_separate);
+		}
+
+		chan->raw[0] = ltc2664_mspan_lut[mspan][1];
+		chan->raw[1] = ltc2664_mspan_lut[mspan][1];
+
+		chan->span = ltc2664_mspan_lut[mspan][0];
+
+		ret = fwnode_property_read_u32_array(child, "adi,output-range-microvolt",
+						     tmp, ARRAY_SIZE(tmp));
+		if (!ret && mspan == LTC2664_MSPAN_SOFTSPAN) {
+			chan->span = ltc2664_set_span(st, tmp[0] / 1000,
+						      tmp[1] / 1000, reg);
+			if (span < 0)
+				return dev_err_probe(dev, span,
+						     "Failed to set span\n");
+
+		}
+
+		ret = fwnode_property_read_u32(child,
+					       "adi,output-range-microamp",
+					       &tmp[0]);
+		if (!ret) {
+			chan->span = ltc2664_set_span(st, 0, tmp[0] / 1000, reg);
+			if (span < 0)
+				return dev_err_probe(dev, span,
+						     "Failed to set span\n");
+		}
+	}
+
+	return 0;
+}
+
+static int ltc2664_setup(struct ltc2664_state *st)
+{
+	const struct ltc2664_chip_info *chip_info = st->chip_info;
+	struct gpio_desc *gpio;
+	int ret, i;
+
+	/* If we have a clr/reset pin, use that to reset the chip. */
+	gpio = devm_gpiod_get_optional(&st->spi->dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(gpio))
+		return dev_err_probe(&st->spi->dev, PTR_ERR(gpio),
+				     "Failed to get reset gpio");
+	if (gpio) {
+		fsleep(1000);
+		gpiod_set_value_cansleep(gpio, 0);
+	}
+
+	/*
+	 * Duplicate the default channel configuration as it can change during
+	 * @ltc2664_channel_config()
+	 */
+	st->iio_channels = devm_kcalloc(&st->spi->dev,
+					chip_info->num_channels,
+					sizeof(struct iio_chan_spec),
+					GFP_KERNEL);
+	if (!st->iio_channels)
+		return -ENOMEM;
+
+	for (i = 0; i < chip_info->num_channels; i++) {
+		st->iio_channels[i] = ltc2664_channel_template;
+		st->iio_channels[i].type = chip_info->measurement_type;
+		st->iio_channels[i].channel = i;
+	}
+
+	ret = ltc2664_channel_config(st);
+	if (ret)
+		return ret;
+
+	return regmap_set_bits(st->regmap, LTC2664_CMD_CONFIG, LTC2664_REF_DISABLE);
+}
+
+static const struct regmap_config ltc2664_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 16,
+	.max_register = LTC2664_CMD_NO_OPERATION,
+};
+
+static const struct iio_info ltc2664_info = {
+	.write_raw = ltc2664_write_raw,
+	.read_raw = ltc2664_read_raw,
+	.read_avail = ltc2664_read_avail,
+	.debugfs_reg_access = ltc2664_reg_access,
+};
+
+static int ltc2664_probe(struct spi_device *spi)
+{
+	static const char * const regulators[] = { "vcc", "iovcc", "v-neg" };
+	const struct ltc2664_chip_info *chip_info;
+	struct device *dev = &spi->dev;
+	struct iio_dev *indio_dev;
+	struct ltc2664_state *st;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	st->spi = spi;
+
+	chip_info = spi_get_device_match_data(spi);
+	if (!chip_info)
+		return -ENODEV;
+
+	st->chip_info = chip_info;
+
+	mutex_init(&st->lock);
+
+	st->regmap = devm_regmap_init_spi(spi, &ltc2664_regmap_config);
+	if (IS_ERR(st->regmap))
+		return dev_err_probe(dev, PTR_ERR(st->regmap),
+				     "Failed to init regmap");
+
+	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulators),
+					     regulators);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable regulators\n");
+
+	ret = devm_regulator_get_enable_read_voltage(dev, "ref");
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Failed to get vref voltage\n");
+	else if (ret == -ENODEV)
+		st->vref_mv = chip_info->internal_vref_mv;
+	else
+		st->vref_mv = ret / 1000;
+
+	ret = ltc2664_setup(st);
+	if (ret)
+		return ret;
+
+	indio_dev->name = chip_info->name;
+	indio_dev->info = &ltc2664_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = st->iio_channels;
+	indio_dev->num_channels = chip_info->num_channels;
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static const struct spi_device_id ltc2664_id[] = {
+	{ "ltc2664", (kernel_ulong_t)&ltc2664_chip },
+	{ "ltc2672", (kernel_ulong_t)&ltc2672_chip },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, ltc2664_id);
+
+static const struct of_device_id ltc2664_of_id[] = {
+	{ .compatible = "adi,ltc2664", .data = &ltc2664_chip },
+	{ .compatible = "adi,ltc2672", .data = &ltc2672_chip },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ltc2664_of_id);
+
+static struct spi_driver ltc2664_driver = {
+	.driver = {
+		.name = "ltc2664",
+		.of_match_table = ltc2664_of_id,
+	},
+	.probe = ltc2664_probe,
+	.id_table = ltc2664_id,
+};
+module_spi_driver(ltc2664_driver);
+
+MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>");
+MODULE_AUTHOR("Kim Seer Paller <kimseer.paller@analog.com>");
+MODULE_DESCRIPTION("Analog Devices LTC2664 and LTC2672 DAC");
+MODULE_LICENSE("GPL");
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v4 3/5] dt-bindings: iio: dac: Add adi,ltc2664.yaml
  2024-06-19  6:49 ` [PATCH v4 3/5] dt-bindings: iio: dac: Add adi,ltc2664.yaml Kim Seer Paller
@ 2024-06-19 17:56   ` Conor Dooley
  2024-06-24 15:13     ` Paller, Kim Seer
  0 siblings, 1 reply; 24+ messages in thread
From: Conor Dooley @ 2024-06-19 17:56 UTC (permalink / raw)
  To: Kim Seer Paller
  Cc: linux-kernel, linux-iio, devicetree, Jonathan Cameron,
	David Lechner, Lars-Peter Clausen, Liam Girdwood, Mark Brown,
	Dimitri Fedrau, Krzysztof Kozlowski, Rob Herring, Conor Dooley,
	Michael Hennerich, Nuno Sá

[-- Attachment #1: Type: text/plain, Size: 7097 bytes --]

On Wed, Jun 19, 2024 at 02:49:02PM +0800, Kim Seer Paller wrote:
> Add documentation for ltc2664.
> 
> Co-developed-by: Michael Hennerich <michael.hennerich@analog.com>
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> ---
>  .../bindings/iio/dac/adi,ltc2664.yaml         | 167 ++++++++++++++++++
>  MAINTAINERS                                   |   8 +
>  2 files changed, 175 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
> new file mode 100644
> index 000000000000..be37700e3b1f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
> @@ -0,0 +1,167 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/dac/adi,ltc2664.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices LTC2664 DAC
> +
> +maintainers:
> +  - Michael Hennerich <michael.hennerich@analog.com>
> +  - Kim Seer Paller <kimseer.paller@analog.com>
> +
> +description: |
> +  Analog Devices LTC2664 4 channel, 12-/16-Bit, +-10V DAC
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/2664fa.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ltc2664
> +
> +  reg:
> +    maxItems: 1
> +
> +  spi-max-frequency:
> +    maximum: 50000000
> +
> +  vcc-supply:
> +    description: Analog Supply Voltage Input.
> +
> +  v-pos-supply:
> +    description: Positive Supply Voltage Input.
> +
> +  v-neg-supply:
> +    description: Negative Supply Voltage Input.
> +
> +  iovcc-supply:
> +    description: Digital Input/Output Supply Voltage.
> +
> +  ref-supply:
> +    description:
> +      Reference Input/Output. The voltage at the REF pin sets the full-scale
> +      range of all channels. If not provided the internal reference is used and
> +      also provided on the VREF pin.
> +
> +  reset-gpios:
> +    description:
> +      Active-low Asynchronous Clear Input. A logic low at this level-triggered
> +      input clears the part to the reset code and range determined by the
> +      hardwired option chosen using the MSPAN pins. The control registers are
> +      cleared to zero.
> +    maxItems: 1
> +
> +  adi,manual-span-operation-config:
> +    description:
> +      This property must mimic the MSPAN pin configurations. By tying the MSPAN
> +      pins (MSP2, MSP1 and MSP0) to GND and/or VCC, any output range can be
> +      hardware-configured with different mid-scale or zero-scale reset options.
> +      The hardware configuration is latched during power on reset for proper
> +      operation.
> +        0 - MPS2=GND, MPS1=GND, MSP0=GND (+-10V, reset to 0V)
> +        1 - MPS2=GND, MPS1=GND, MSP0=VCC (+-5V, reset to 0V)
> +        2 - MPS2=GND, MPS1=VCC, MSP0=GND (+-2.5V, reset to 0V)
> +        3 - MPS2=GND, MPS1=VCC, MSP0=VCC (0V to 10, reset to 0V)
> +        4 - MPS2=VCC, MPS1=GND, MSP0=GND (0V to 10V, reset to 5V)
> +        5 - MPS2=VCC, MPS1=GND, MSP0=VCC (0V to 5V, reset to 0V)
> +        6 - MPS2=VCC, MPS1=VCC, MSP0=GND (0V to 5V, reset to 2.5V)
> +        7 - MPS2=VCC, MPS1=VCC, MSP0=VCC (0V to 5V, reset to 0V, enables SoftSpan)
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1, 2, 3, 4, 5, 6, 7]

Can you explain why this property is required, when below there's one
that sets the ranges in microvolts? Isn't the only new information that
this provides the reset values (in a few cases that it is not 0).
What am I missing?

> +    default: 7
> +
> +  io-channels:
> +    description:
> +      ADC channel to monitor voltages and temperature at the MUXOUT pin.
> +    maxItems: 1
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +patternProperties:
> +  "^channel@[0-3]$":
> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      reg:
> +        description: The channel number representing the DAC output channel.
> +        maximum: 3
> +
> +      adi,toggle-mode:
> +        description:
> +          Set the channel as a toggle enabled channel. Toggle operation enables
> +          fast switching of a DAC output between two different DAC codes without
> +          any SPI transaction.
> +        type: boolean
> +
> +      adi,output-range-microvolt:
> +        description: Specify the channel output full scale range.
> +        oneOf:
> +          - items:
> +              - const: 0
> +              - enum: [5000000, 10000000]
> +          - items:
> +              - const: -5000000
> +              - const: 5000000
> +          - items:
> +              - const: -10000000
> +              - const: 10000000
> +          - items:
> +              - const: -2500000
> +              - const: 2500000
> +
> +    required:
> +      - reg
> +      - adi,output-range-microvolt
> +
> +required:
> +  - compatible
> +  - reg
> +  - spi-max-frequency
> +  - vcc-supply
> +  - iovcc-supply
> +  - v-pos-supply
> +  - v-neg-supply
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        dac@0 {
> +            compatible = "adi,ltc2664";
> +            reg = <0>;
> +            spi-max-frequency = <10000000>;
> +
> +            vcc-supply = <&vcc>;
> +            iovcc-supply = <&vcc>;
> +            ref-supply = <&vref>;
> +            v-pos-supply = <&vpos>;
> +            v-neg-supply = <&vneg>;
> +
> +            io-channels = <&adc 0>;
> +
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            channel@0 {
> +                    reg = <0>;
> +                    adi,toggle-mode;
> +                    adi,output-range-microvolt = <(-10000000) 10000000>;
> +            };
> +
> +            channel@1 {
> +                    reg = <1>;
> +                    adi,output-range-microvolt = <0 10000000>;
> +            };
> +        };
> +    };
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index be590c462d91..849800d9cbf7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13074,6 +13074,14 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/iio/dac/lltc,ltc1660.yaml
>  F:	drivers/iio/dac/ltc1660.c
>  
> +LTC2664 IIO DAC DRIVER
> +M:	Michael Hennerich <michael.hennerich@analog.com>
> +M:	Kim Seer Paller <kimseer.paller@analog.com>
> +L:	linux-iio@vger.kernel.org
> +S:	Supported
> +W:	https://ez.analog.com/linux-software-drivers
> +F:	Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
> +
>  LTC2688 IIO DAC DRIVER
>  M:	Nuno Sá <nuno.sa@analog.com>
>  L:	linux-iio@vger.kernel.org
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v4 4/5] dt-bindings: iio: dac: Add adi,ltc2672.yaml
  2024-06-19  6:49 ` [PATCH v4 4/5] dt-bindings: iio: dac: Add adi,ltc2672.yaml Kim Seer Paller
@ 2024-06-19 17:57   ` Conor Dooley
  2024-06-23 13:43     ` Jonathan Cameron
  0 siblings, 1 reply; 24+ messages in thread
From: Conor Dooley @ 2024-06-19 17:57 UTC (permalink / raw)
  To: Kim Seer Paller
  Cc: linux-kernel, linux-iio, devicetree, Jonathan Cameron,
	David Lechner, Lars-Peter Clausen, Liam Girdwood, Mark Brown,
	Dimitri Fedrau, Krzysztof Kozlowski, Rob Herring, Conor Dooley,
	Michael Hennerich, Nuno Sá

[-- Attachment #1: Type: text/plain, Size: 883 bytes --]

On Wed, Jun 19, 2024 at 02:49:03PM +0800, Kim Seer Paller wrote:
> +patternProperties:
> +  "^channel@[0-4]$":
> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      reg:
> +        description: The channel number representing the DAC output channel.
> +        maximum: 4
> +
> +      adi,toggle-mode:
> +        description:
> +          Set the channel as a toggle enabled channel. Toggle operation enables
> +          fast switching of a DAC output between two different DAC codes without
> +          any SPI transaction.
> +        type: boolean
> +
> +      adi,output-range-microamp:
> +        description: Specify the channel output full scale range.
> +        enum: [3125000, 6250000, 12500000, 25000000, 50000000, 100000000,
> +               200000000, 300000000]

IIO folks, is this sort of thing common/likely to exist on other DACs?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* RE: [PATCH v4 5/5] iio: dac: ltc2664: Add driver for LTC2664 and LTC2672
  2024-06-19  6:49 ` [PATCH v4 5/5] iio: dac: ltc2664: Add driver for LTC2664 and LTC2672 Kim Seer Paller
@ 2024-06-20  0:00   ` Paller, Kim Seer
  2024-06-23 13:50   ` Jonathan Cameron
  2024-06-26 11:14   ` Nuno Sá
  2 siblings, 0 replies; 24+ messages in thread
From: Paller, Kim Seer @ 2024-06-20  0:00 UTC (permalink / raw)
  To: linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org,
	devicetree@vger.kernel.org
  Cc: Jonathan Cameron, David Lechner, Lars-Peter Clausen,
	Liam Girdwood, Mark Brown, Dimitri Fedrau, Krzysztof Kozlowski,
	Rob Herring, Conor Dooley, Hennerich, Michael, Nuno Sá



> -----Original Message-----
> From: Kim Seer Paller <kimseer.paller@analog.com>
> Sent: Wednesday, June 19, 2024 2:49 PM
> To: linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org;
> devicetree@vger.kernel.org
> Cc: Jonathan Cameron <jic23@kernel.org>; David Lechner
> <dlechner@baylibre.com>; Lars-Peter Clausen <lars@metafoo.de>; Liam
> Girdwood <lgirdwood@gmail.com>; Mark Brown <broonie@kernel.org>;
> Dimitri Fedrau <dima.fedrau@gmail.com>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Rob Herring <robh@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; Hennerich, Michael
> <Michael.Hennerich@analog.com>; Nuno Sá <noname.nuno@gmail.com>;
> Paller, Kim Seer <KimSeer.Paller@analog.com>
> Subject: [PATCH v4 5/5] iio: dac: ltc2664: Add driver for LTC2664 and LTC2672
> 
> LTC2664 4 channel, 12-/16-Bit Voltage Output SoftSpan DAC
> LTC2672 5 channel, 12-/16-Bit Current Output Softspan DAC
> 
> Co-developed-by: Michael Hennerich <michael.hennerich@analog.com>
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> ---
>  MAINTAINERS               |   1 +
>  drivers/iio/dac/Kconfig   |  11 +
>  drivers/iio/dac/Makefile  |   1 +
>  drivers/iio/dac/ltc2664.c | 755 ++++++++++++++++++++++++++++++++++++++
>  4 files changed, 768 insertions(+)
>  create mode 100644 drivers/iio/dac/ltc2664.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f4a5b5bc8ccc..7a02d9a196fb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13082,6 +13082,7 @@ S:	Supported
>  W:	https://ez.analog.com/linux-software-drivers
>  F:	Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
>  F:	Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml
> +F:	drivers/iio/dac/ltc2664.c
> 
>  LTC2688 IIO DAC DRIVER
>  M:	Nuno Sá <nuno.sa@analog.com>
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig index
> fb48dddbcc20..3a7691db3998 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -371,6 +371,17 @@ config LTC2632
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called ltc2632.
> 
> +config LTC2664
> +	tristate "Analog Devices LTC2664 and LTC2672 DAC SPI driver"
> +	depends on SPI
> +	select REGMAP
> +	help
> +	  Say yes here to build support for Analog Devices
> +	  LTC2664 and LTC2672 converters (DAC).
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ltc2664.
> +
>  config M62332
>  	tristate "Mitsubishi M62332 DAC driver"
>  	depends on I2C
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile index
> 8432a81a19dc..2cf148f16306 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -37,6 +37,7 @@ obj-$(CONFIG_DS4424) += ds4424.o
>  obj-$(CONFIG_LPC18XX_DAC) += lpc18xx_dac.o
>  obj-$(CONFIG_LTC1660) += ltc1660.o
>  obj-$(CONFIG_LTC2632) += ltc2632.o
> +obj-$(CONFIG_LTC2664) += ltc2664.o
>  obj-$(CONFIG_LTC2688) += ltc2688.o
>  obj-$(CONFIG_M62332) += m62332.o
>  obj-$(CONFIG_MAX517) += max517.o
> diff --git a/drivers/iio/dac/ltc2664.c b/drivers/iio/dac/ltc2664.c new file mode
> 100644 index 000000000000..9b73b9c6a7a7
> --- /dev/null
> +++ b/drivers/iio/dac/ltc2664.c
> @@ -0,0 +1,755 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * LTC2664 4 channel, 12-/16-Bit Voltage Output SoftSpan DAC driver
> + * LTC2672 5 channel, 12-/16-Bit Current Output Softspan DAC driver
> + *
> + * Copyright 2024 Analog Devices Inc.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/cleanup.h>
> +#include <linux/device.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/kernel.h>
> +#include <linux/math64.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/mutex.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +
> +#define LTC2664_CMD_WRITE_N(n)		(0x00 + (n))
> +#define LTC2664_CMD_UPDATE_N(n)		(0x10 + (n))
> +#define LTC2664_CMD_WRITE_N_UPDATE_ALL	0x20
> +#define LTC2664_CMD_WRITE_N_UPDATE_N(n)	(0x30 + (n))
> +#define LTC2664_CMD_POWER_DOWN_N(n)	(0x40 + (n))
> +#define LTC2664_CMD_POWER_DOWN_ALL	0x50
> +#define LTC2664_CMD_SPAN_N(n)		(0x60 + (n))
> +#define LTC2664_CMD_CONFIG		0x70
> +#define LTC2664_CMD_MUX			0xB0
> +#define LTC2664_CMD_TOGGLE_SEL		0xC0
> +#define LTC2664_CMD_GLOBAL_TOGGLE	0xD0
> +#define LTC2664_CMD_NO_OPERATION	0xF0
> +#define LTC2664_REF_DISABLE		0x0001
> +#define LTC2664_MSPAN_SOFTSPAN		7
> +
> +#define LTC2672_MAX_CHANNEL		5
> +#define LTC2672_MAX_SPAN		7
> +#define LTC2672_SCALE_MULTIPLIER(n)	(50 * BIT(n))
> +
> +enum ltc2664_ids {
> +	LTC2664,
> +	LTC2672,
> +};
> +
> +enum {
> +	LTC2664_SPAN_RANGE_0V_5V,
> +	LTC2664_SPAN_RANGE_0V_10V,
> +	LTC2664_SPAN_RANGE_M5V_5V,
> +	LTC2664_SPAN_RANGE_M10V_10V,
> +	LTC2664_SPAN_RANGE_M2V5_2V5,
> +};
> +
> +enum {
> +	LTC2664_INPUT_A,
> +	LTC2664_INPUT_B,
> +	LTC2664_INPUT_B_AVAIL,
> +	LTC2664_POWERDOWN,
> +	LTC2664_POWERDOWN_MODE,
> +	LTC2664_TOGGLE_EN,
> +	LTC2664_GLOBAL_TOGGLE,
> +};
> +
> +static const u16 ltc2664_mspan_lut[8][2] = {
> +	{ LTC2664_SPAN_RANGE_M10V_10V, 32768 }, /* MPS2=0, MPS1=0,
> MSP0=0 (0)*/
> +	{ LTC2664_SPAN_RANGE_M5V_5V, 32768 }, /* MPS2=0, MPS1=0,
> MSP0=1 (1)*/
> +	{ LTC2664_SPAN_RANGE_M2V5_2V5, 32768 }, /* MPS2=0, MPS1=1,
> MSP0=0 (2)*/
> +	{ LTC2664_SPAN_RANGE_0V_10V, 0 }, /* MPS2=0, MPS1=1, MSP0=1
> (3)*/
> +	{ LTC2664_SPAN_RANGE_0V_10V, 32768 }, /* MPS2=1, MPS1=0,
> MSP0=0 (4)*/
> +	{ LTC2664_SPAN_RANGE_0V_5V, 0 }, /* MPS2=1, MPS1=0, MSP0=1
> (5)*/
> +	{ LTC2664_SPAN_RANGE_0V_5V, 32768 }, /* MPS2=1, MPS1=1,
> MSP0=0 (6)*/
> +	{ LTC2664_SPAN_RANGE_0V_5V, 0 } /* MPS2=1, MPS1=1, MSP0=1
> (7)*/ };
> +
> +struct ltc2664_state;
> +
> +struct ltc2664_chip_info {
> +	enum ltc2664_ids id;

Apologies for the oversight, forgot to remove both the id field and the
corresponding enum id.

> +	const char *name;
> +	int (*scale_get)(const struct ltc2664_state *st, int c);
> +	int (*offset_get)(const struct ltc2664_state *st, int c);
> +	int measurement_type;
> +	unsigned int num_channels;
> +	const struct iio_chan_spec *iio_chan;
> +	const int (*span_helper)[2];
> +	unsigned int num_span;
> +	unsigned int internal_vref_mv;
> +	bool manual_span_support;
> +	bool rfsadj_support;


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v4 4/5] dt-bindings: iio: dac: Add adi,ltc2672.yaml
  2024-06-19 17:57   ` Conor Dooley
@ 2024-06-23 13:43     ` Jonathan Cameron
  2024-06-23 14:03       ` Conor Dooley
  2024-06-24 15:26       ` Paller, Kim Seer
  0 siblings, 2 replies; 24+ messages in thread
From: Jonathan Cameron @ 2024-06-23 13:43 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Kim Seer Paller, linux-kernel, linux-iio, devicetree,
	David Lechner, Lars-Peter Clausen, Liam Girdwood, Mark Brown,
	Dimitri Fedrau, Krzysztof Kozlowski, Rob Herring, Conor Dooley,
	Michael Hennerich, Nuno Sá

On Wed, 19 Jun 2024 18:57:59 +0100
Conor Dooley <conor@kernel.org> wrote:

> On Wed, Jun 19, 2024 at 02:49:03PM +0800, Kim Seer Paller wrote:
> > +patternProperties:
> > +  "^channel@[0-4]$":
> > +    type: object
> > +    additionalProperties: false
> > +
> > +    properties:
> > +      reg:
> > +        description: The channel number representing the DAC output channel.
> > +        maximum: 4
> > +
> > +      adi,toggle-mode:
> > +        description:
> > +          Set the channel as a toggle enabled channel. Toggle operation enables
> > +          fast switching of a DAC output between two different DAC codes without
> > +          any SPI transaction.
> > +        type: boolean
> > +
> > +      adi,output-range-microamp:
> > +        description: Specify the channel output full scale range.
> > +        enum: [3125000, 6250000, 12500000, 25000000, 50000000, 100000000,
> > +               200000000, 300000000]  
> 
> IIO folks, is this sort of thing common/likely to exist on other DACs?

Fair point. It is probably time to conclude this is at least moderately common
and generalize it - which will need a dac.yaml similar to the one we have for
ADCs in adc/adc.yaml.  That will need to make this a per channel node
property (same as the adc ones).
 
I'd also expect it to always take 2 values. In many cases the first will be 0
but that is fine.

Jonathan


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v4 5/5] iio: dac: ltc2664: Add driver for LTC2664 and LTC2672
  2024-06-19  6:49 ` [PATCH v4 5/5] iio: dac: ltc2664: Add driver for LTC2664 and LTC2672 Kim Seer Paller
  2024-06-20  0:00   ` Paller, Kim Seer
@ 2024-06-23 13:50   ` Jonathan Cameron
  2024-06-26 11:14   ` Nuno Sá
  2 siblings, 0 replies; 24+ messages in thread
From: Jonathan Cameron @ 2024-06-23 13:50 UTC (permalink / raw)
  To: Kim Seer Paller
  Cc: linux-kernel, linux-iio, devicetree, David Lechner,
	Lars-Peter Clausen, Liam Girdwood, Mark Brown, Dimitri Fedrau,
	Krzysztof Kozlowski, Rob Herring, Conor Dooley, Michael Hennerich,
	Nuno Sá

On Wed, 19 Jun 2024 14:49:04 +0800
Kim Seer Paller <kimseer.paller@analog.com> wrote:

> LTC2664 4 channel, 12-/16-Bit Voltage Output SoftSpan DAC
> LTC2672 5 channel, 12-/16-Bit Current Output Softspan DAC
> 
> Co-developed-by: Michael Hennerich <michael.hennerich@analog.com>
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>

Other than oustanding questions on the binding this looks good
to me.

Jonathan


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v4 4/5] dt-bindings: iio: dac: Add adi,ltc2672.yaml
  2024-06-23 13:43     ` Jonathan Cameron
@ 2024-06-23 14:03       ` Conor Dooley
  2024-06-23 16:20         ` Jonathan Cameron
  2024-06-24 15:26       ` Paller, Kim Seer
  1 sibling, 1 reply; 24+ messages in thread
From: Conor Dooley @ 2024-06-23 14:03 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Kim Seer Paller, linux-kernel, linux-iio, devicetree,
	David Lechner, Lars-Peter Clausen, Liam Girdwood, Mark Brown,
	Dimitri Fedrau, Krzysztof Kozlowski, Rob Herring, Conor Dooley,
	Michael Hennerich, Nuno Sá

[-- Attachment #1: Type: text/plain, Size: 1624 bytes --]

On Sun, Jun 23, 2024 at 02:43:39PM +0100, Jonathan Cameron wrote:
> On Wed, 19 Jun 2024 18:57:59 +0100
> Conor Dooley <conor@kernel.org> wrote:
> 
> > On Wed, Jun 19, 2024 at 02:49:03PM +0800, Kim Seer Paller wrote:
> > > +patternProperties:
> > > +  "^channel@[0-4]$":
> > > +    type: object
> > > +    additionalProperties: false
> > > +
> > > +    properties:
> > > +      reg:
> > > +        description: The channel number representing the DAC output channel.
> > > +        maximum: 4
> > > +
> > > +      adi,toggle-mode:
> > > +        description:
> > > +          Set the channel as a toggle enabled channel. Toggle operation enables
> > > +          fast switching of a DAC output between two different DAC codes without
> > > +          any SPI transaction.
> > > +        type: boolean
> > > +
> > > +      adi,output-range-microamp:
> > > +        description: Specify the channel output full scale range.
> > > +        enum: [3125000, 6250000, 12500000, 25000000, 50000000, 100000000,
> > > +               200000000, 300000000]  
> > 
> > IIO folks, is this sort of thing common/likely to exist on other DACs?
> 
> Fair point. It is probably time to conclude this is at least moderately common
> and generalize it - which will need a dac.yaml similar to the one we have for
> ADCs in adc/adc.yaml.  That will need to make this a per channel node
> property (same as the adc ones).

Looks like it is already per channel node?

> I'd also expect it to always take 2 values. In many cases the first will be 0
> but that is fine.

What would that first value represent?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v4 4/5] dt-bindings: iio: dac: Add adi,ltc2672.yaml
  2024-06-23 14:03       ` Conor Dooley
@ 2024-06-23 16:20         ` Jonathan Cameron
  0 siblings, 0 replies; 24+ messages in thread
From: Jonathan Cameron @ 2024-06-23 16:20 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Kim Seer Paller, linux-kernel, linux-iio, devicetree,
	David Lechner, Lars-Peter Clausen, Liam Girdwood, Mark Brown,
	Dimitri Fedrau, Krzysztof Kozlowski, Rob Herring, Conor Dooley,
	Michael Hennerich, Nuno Sá

On Sun, 23 Jun 2024 15:03:47 +0100
Conor Dooley <conor@kernel.org> wrote:

> On Sun, Jun 23, 2024 at 02:43:39PM +0100, Jonathan Cameron wrote:
> > On Wed, 19 Jun 2024 18:57:59 +0100
> > Conor Dooley <conor@kernel.org> wrote:
> >   
> > > On Wed, Jun 19, 2024 at 02:49:03PM +0800, Kim Seer Paller wrote:  
> > > > +patternProperties:
> > > > +  "^channel@[0-4]$":
> > > > +    type: object
> > > > +    additionalProperties: false
> > > > +
> > > > +    properties:
> > > > +      reg:
> > > > +        description: The channel number representing the DAC output channel.
> > > > +        maximum: 4
> > > > +
> > > > +      adi,toggle-mode:
> > > > +        description:
> > > > +          Set the channel as a toggle enabled channel. Toggle operation enables
> > > > +          fast switching of a DAC output between two different DAC codes without
> > > > +          any SPI transaction.
> > > > +        type: boolean
> > > > +
> > > > +      adi,output-range-microamp:
> > > > +        description: Specify the channel output full scale range.
> > > > +        enum: [3125000, 6250000, 12500000, 25000000, 50000000, 100000000,
> > > > +               200000000, 300000000]    
> > > 
> > > IIO folks, is this sort of thing common/likely to exist on other DACs?  
> > 
> > Fair point. It is probably time to conclude this is at least moderately common
> > and generalize it - which will need a dac.yaml similar to the one we have for
> > ADCs in adc/adc.yaml.  That will need to make this a per channel node
> > property (same as the adc ones).  
> 
> Looks like it is already per channel node?

Absolutely - but that is a bit fiddlier to do in a generic file so I just
meant make sure to enforce that in a similar fashion to adc.yaml.
> 
> > I'd also expect it to always take 2 values. In many cases the first will be 0
> > but that is fine.  
> 
> What would that first value represent?
Hmm. For voltage equivalent they are often not zero based so it would be the
negative. I have no idea if there are current dacs that act as both sources and sinks...
So maybe voltage one which should be done in this series as well needs to be 2 value
and current variant maybe not.

Jonathan



^ permalink raw reply	[flat|nested] 24+ messages in thread

* RE: [PATCH v4 3/5] dt-bindings: iio: dac: Add adi,ltc2664.yaml
  2024-06-19 17:56   ` Conor Dooley
@ 2024-06-24 15:13     ` Paller, Kim Seer
  2024-06-24 16:13       ` Conor Dooley
  2024-06-24 16:48       ` David Lechner
  0 siblings, 2 replies; 24+ messages in thread
From: Paller, Kim Seer @ 2024-06-24 15:13 UTC (permalink / raw)
  To: Conor Dooley
  Cc: linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org,
	devicetree@vger.kernel.org, Jonathan Cameron, David Lechner,
	Lars-Peter Clausen, Liam Girdwood, Mark Brown, Dimitri Fedrau,
	Krzysztof Kozlowski, Rob Herring, Conor Dooley,
	Hennerich, Michael, Nuno Sá



> -----Original Message-----
> From: Conor Dooley <conor@kernel.org>
> Sent: Thursday, June 20, 2024 1:57 AM
> To: Paller, Kim Seer <KimSeer.Paller@analog.com>
> Cc: linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org;
> devicetree@vger.kernel.org; Jonathan Cameron <jic23@kernel.org>; David
> Lechner <dlechner@baylibre.com>; Lars-Peter Clausen <lars@metafoo.de>;
> Liam Girdwood <lgirdwood@gmail.com>; Mark Brown <broonie@kernel.org>;
> Dimitri Fedrau <dima.fedrau@gmail.com>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Rob Herring <robh@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; Hennerich, Michael
> <Michael.Hennerich@analog.com>; Nuno Sá <noname.nuno@gmail.com>
> Subject: Re: [PATCH v4 3/5] dt-bindings: iio: dac: Add adi,ltc2664.yaml
> 
> [External]
> 
> On Wed, Jun 19, 2024 at 02:49:02PM +0800, Kim Seer Paller wrote:
> > Add documentation for ltc2664.
> >
> > Co-developed-by: Michael Hennerich <michael.hennerich@analog.com>
> > Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> > ---
> >  .../bindings/iio/dac/adi,ltc2664.yaml         | 167 ++++++++++++++++++
> >  MAINTAINERS                                   |   8 +
> >  2 files changed, 175 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
> > b/Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
> > new file mode 100644
> > index 000000000000..be37700e3b1f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
> > @@ -0,0 +1,167 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/dac/adi,ltc2664.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Analog Devices LTC2664 DAC
> > +
> > +maintainers:
> > +  - Michael Hennerich <michael.hennerich@analog.com>
> > +  - Kim Seer Paller <kimseer.paller@analog.com>
> > +
> > +description: |
> > +  Analog Devices LTC2664 4 channel, 12-/16-Bit, +-10V DAC
> > +
> > +https://www.analog.com/media/en/technical-documentation/data-sheets/2
> > +664fa.pdf
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - adi,ltc2664
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  spi-max-frequency:
> > +    maximum: 50000000
> > +
> > +  vcc-supply:
> > +    description: Analog Supply Voltage Input.
> > +
> > +  v-pos-supply:
> > +    description: Positive Supply Voltage Input.
> > +
> > +  v-neg-supply:
> > +    description: Negative Supply Voltage Input.
> > +
> > +  iovcc-supply:
> > +    description: Digital Input/Output Supply Voltage.
> > +
> > +  ref-supply:
> > +    description:
> > +      Reference Input/Output. The voltage at the REF pin sets the full-scale
> > +      range of all channels. If not provided the internal reference is used and
> > +      also provided on the VREF pin.
> > +
> > +  reset-gpios:
> > +    description:
> > +      Active-low Asynchronous Clear Input. A logic low at this level-triggered
> > +      input clears the part to the reset code and range determined by the
> > +      hardwired option chosen using the MSPAN pins. The control registers are
> > +      cleared to zero.
> > +    maxItems: 1
> > +
> > +  adi,manual-span-operation-config:
> > +    description:
> > +      This property must mimic the MSPAN pin configurations. By tying the
> MSPAN
> > +      pins (MSP2, MSP1 and MSP0) to GND and/or VCC, any output range can
> be
> > +      hardware-configured with different mid-scale or zero-scale reset options.
> > +      The hardware configuration is latched during power on reset for proper
> > +      operation.
> > +        0 - MPS2=GND, MPS1=GND, MSP0=GND (+-10V, reset to 0V)
> > +        1 - MPS2=GND, MPS1=GND, MSP0=VCC (+-5V, reset to 0V)
> > +        2 - MPS2=GND, MPS1=VCC, MSP0=GND (+-2.5V, reset to 0V)
> > +        3 - MPS2=GND, MPS1=VCC, MSP0=VCC (0V to 10, reset to 0V)
> > +        4 - MPS2=VCC, MPS1=GND, MSP0=GND (0V to 10V, reset to 5V)
> > +        5 - MPS2=VCC, MPS1=GND, MSP0=VCC (0V to 5V, reset to 0V)
> > +        6 - MPS2=VCC, MPS1=VCC, MSP0=GND (0V to 5V, reset to 2.5V)
> > +        7 - MPS2=VCC, MPS1=VCC, MSP0=VCC (0V to 5V, reset to 0V, enables
> SoftSpan)
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [0, 1, 2, 3, 4, 5, 6, 7]
> 
> Can you explain why this property is required, when below there's one that sets
> the ranges in microvolts? Isn't the only new information that this provides the
> reset values (in a few cases that it is not 0).
> What am I missing?

For specifying output range and reset options without relying on software initialization
routines, and also for enabling the softspan feature, I think this property seems essential.

> > +    default: 7
> > +
> > +  io-channels:
> > +    description:
> > +      ADC channel to monitor voltages and temperature at the MUXOUT pin.
> > +    maxItems: 1
> > +
> > +  '#address-cells':
> > +    const: 1
> > +
> > +  '#size-cells':
> > +    const: 0
> > +
> > +patternProperties:
> > +  "^channel@[0-3]$":
> > +    type: object
> > +    additionalProperties: false
> > +
> > +    properties:
> > +      reg:
> > +        description: The channel number representing the DAC output channel.
> > +        maximum: 3
> > +
> > +      adi,toggle-mode:
> > +        description:
> > +          Set the channel as a toggle enabled channel. Toggle operation enables
> > +          fast switching of a DAC output between two different DAC codes
> without
> > +          any SPI transaction.
> > +        type: boolean
> > +
> > +      adi,output-range-microvolt:
> > +        description: Specify the channel output full scale range.
> > +        oneOf:
> > +          - items:
> > +              - const: 0
> > +              - enum: [5000000, 10000000]
> > +          - items:
> > +              - const: -5000000
> > +              - const: 5000000
> > +          - items:
> > +              - const: -10000000
> > +              - const: 10000000
> > +          - items:
> > +              - const: -2500000
> > +              - const: 2500000
> > +
> > +    required:
> > +      - reg
> > +      - adi,output-range-microvolt
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - spi-max-frequency
> > +  - vcc-supply
> > +  - iovcc-supply
> > +  - v-pos-supply
> > +  - v-neg-supply
> > +
> > +allOf:
> > +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    spi {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +        dac@0 {
> > +            compatible = "adi,ltc2664";
> > +            reg = <0>;
> > +            spi-max-frequency = <10000000>;
> > +
> > +            vcc-supply = <&vcc>;
> > +            iovcc-supply = <&vcc>;
> > +            ref-supply = <&vref>;
> > +            v-pos-supply = <&vpos>;
> > +            v-neg-supply = <&vneg>;
> > +
> > +            io-channels = <&adc 0>;
> > +
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +            channel@0 {
> > +                    reg = <0>;
> > +                    adi,toggle-mode;
> > +                    adi,output-range-microvolt = <(-10000000) 10000000>;
> > +            };
> > +
> > +            channel@1 {
> > +                    reg = <1>;
> > +                    adi,output-range-microvolt = <0 10000000>;
> > +            };
> > +        };
> > +    };
> > +...
> > diff --git a/MAINTAINERS b/MAINTAINERS index
> > be590c462d91..849800d9cbf7 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -13074,6 +13074,14 @@ S:	Maintained
> >  F:	Documentation/devicetree/bindings/iio/dac/lltc,ltc1660.yaml
> >  F:	drivers/iio/dac/ltc1660.c
> >
> > +LTC2664 IIO DAC DRIVER
> > +M:	Michael Hennerich <michael.hennerich@analog.com>
> > +M:	Kim Seer Paller <kimseer.paller@analog.com>
> > +L:	linux-iio@vger.kernel.org
> > +S:	Supported
> > +W:	https://ez.analog.com/linux-software-drivers
> > +F:	Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
> > +
> >  LTC2688 IIO DAC DRIVER
> >  M:	Nuno Sá <nuno.sa@analog.com>
> >  L:	linux-iio@vger.kernel.org
> > --
> > 2.34.1
> >

^ permalink raw reply	[flat|nested] 24+ messages in thread

* RE: [PATCH v4 4/5] dt-bindings: iio: dac: Add adi,ltc2672.yaml
  2024-06-23 13:43     ` Jonathan Cameron
  2024-06-23 14:03       ` Conor Dooley
@ 2024-06-24 15:26       ` Paller, Kim Seer
  2024-06-24 17:00         ` Conor Dooley
  1 sibling, 1 reply; 24+ messages in thread
From: Paller, Kim Seer @ 2024-06-24 15:26 UTC (permalink / raw)
  To: Jonathan Cameron, Conor Dooley
  Cc: linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org,
	devicetree@vger.kernel.org, David Lechner, Lars-Peter Clausen,
	Liam Girdwood, Mark Brown, Dimitri Fedrau, Krzysztof Kozlowski,
	Rob Herring, Conor Dooley, Hennerich, Michael, Nuno Sá



> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Sunday, June 23, 2024 9:44 PM
> To: Conor Dooley <conor@kernel.org>
> Cc: Paller, Kim Seer <KimSeer.Paller@analog.com>; linux-
> kernel@vger.kernel.org; linux-iio@vger.kernel.org; devicetree@vger.kernel.org;
> David Lechner <dlechner@baylibre.com>; Lars-Peter Clausen
> <lars@metafoo.de>; Liam Girdwood <lgirdwood@gmail.com>; Mark Brown
> <broonie@kernel.org>; Dimitri Fedrau <dima.fedrau@gmail.com>; Krzysztof
> Kozlowski <krzk+dt@kernel.org>; Rob Herring <robh@kernel.org>; Conor
> Dooley <conor+dt@kernel.org>; Hennerich, Michael
> <Michael.Hennerich@analog.com>; Nuno Sá <noname.nuno@gmail.com>
> Subject: Re: [PATCH v4 4/5] dt-bindings: iio: dac: Add adi,ltc2672.yaml
> 
> [External]
> 
> On Wed, 19 Jun 2024 18:57:59 +0100
> Conor Dooley <conor@kernel.org> wrote:
> 
> > On Wed, Jun 19, 2024 at 02:49:03PM +0800, Kim Seer Paller wrote:
> > > +patternProperties:
> > > +  "^channel@[0-4]$":
> > > +    type: object
> > > +    additionalProperties: false
> > > +
> > > +    properties:
> > > +      reg:
> > > +        description: The channel number representing the DAC output
> channel.
> > > +        maximum: 4
> > > +
> > > +      adi,toggle-mode:
> > > +        description:
> > > +          Set the channel as a toggle enabled channel. Toggle operation
> enables
> > > +          fast switching of a DAC output between two different DAC codes
> without
> > > +          any SPI transaction.
> > > +        type: boolean
> > > +
> > > +      adi,output-range-microamp:
> > > +        description: Specify the channel output full scale range.
> > > +        enum: [3125000, 6250000, 12500000, 25000000, 50000000,
> 100000000,
> > > +               200000000, 300000000]
> >
> > IIO folks, is this sort of thing common/likely to exist on other DACs?
> 
> Fair point. It is probably time to conclude this is at least moderately common
> and generalize it - which will need a dac.yaml similar to the one we have for
> ADCs in adc/adc.yaml.  That will need to make this a per channel node property
> (same as the adc ones).

Should I proceed with generalizing common DAC properties in this series and does
this mean somehow removing these common properties from existing DAC bindings?

> 
> I'd also expect it to always take 2 values. In many cases the first will be 0 but
> that is fine.
> 
> Jonathan


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v4 3/5] dt-bindings: iio: dac: Add adi,ltc2664.yaml
  2024-06-24 15:13     ` Paller, Kim Seer
@ 2024-06-24 16:13       ` Conor Dooley
  2024-06-24 16:48       ` David Lechner
  1 sibling, 0 replies; 24+ messages in thread
From: Conor Dooley @ 2024-06-24 16:13 UTC (permalink / raw)
  To: Paller, Kim Seer
  Cc: linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org,
	devicetree@vger.kernel.org, Jonathan Cameron, David Lechner,
	Lars-Peter Clausen, Liam Girdwood, Mark Brown, Dimitri Fedrau,
	Krzysztof Kozlowski, Rob Herring, Conor Dooley,
	Hennerich, Michael, Nuno Sá

[-- Attachment #1: Type: text/plain, Size: 5509 bytes --]

On Mon, Jun 24, 2024 at 03:13:56PM +0000, Paller, Kim Seer wrote:
> 
> 
> > -----Original Message-----
> > From: Conor Dooley <conor@kernel.org>
> > Sent: Thursday, June 20, 2024 1:57 AM
> > To: Paller, Kim Seer <KimSeer.Paller@analog.com>
> > Cc: linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org;
> > devicetree@vger.kernel.org; Jonathan Cameron <jic23@kernel.org>; David
> > Lechner <dlechner@baylibre.com>; Lars-Peter Clausen <lars@metafoo.de>;
> > Liam Girdwood <lgirdwood@gmail.com>; Mark Brown <broonie@kernel.org>;
> > Dimitri Fedrau <dima.fedrau@gmail.com>; Krzysztof Kozlowski
> > <krzk+dt@kernel.org>; Rob Herring <robh@kernel.org>; Conor Dooley
> > <conor+dt@kernel.org>; Hennerich, Michael
> > <Michael.Hennerich@analog.com>; Nuno Sá <noname.nuno@gmail.com>
> > Subject: Re: [PATCH v4 3/5] dt-bindings: iio: dac: Add adi,ltc2664.yaml
> > 
> > [External]
> > 
> > On Wed, Jun 19, 2024 at 02:49:02PM +0800, Kim Seer Paller wrote:
> > > Add documentation for ltc2664.
> > >
> > > Co-developed-by: Michael Hennerich <michael.hennerich@analog.com>
> > > Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> > > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> > > ---
> > >  .../bindings/iio/dac/adi,ltc2664.yaml         | 167 ++++++++++++++++++
> > >  MAINTAINERS                                   |   8 +
> > >  2 files changed, 175 insertions(+)
> > >  create mode 100644
> > > Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
> > > b/Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
> > > new file mode 100644
> > > index 000000000000..be37700e3b1f
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
> > > @@ -0,0 +1,167 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/iio/dac/adi,ltc2664.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Analog Devices LTC2664 DAC
> > > +
> > > +maintainers:
> > > +  - Michael Hennerich <michael.hennerich@analog.com>
> > > +  - Kim Seer Paller <kimseer.paller@analog.com>
> > > +
> > > +description: |
> > > +  Analog Devices LTC2664 4 channel, 12-/16-Bit, +-10V DAC
> > > +
> > > +https://www.analog.com/media/en/technical-documentation/data-sheets/2
> > > +664fa.pdf
> > > +
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - adi,ltc2664
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  spi-max-frequency:
> > > +    maximum: 50000000
> > > +
> > > +  vcc-supply:
> > > +    description: Analog Supply Voltage Input.
> > > +
> > > +  v-pos-supply:
> > > +    description: Positive Supply Voltage Input.
> > > +
> > > +  v-neg-supply:
> > > +    description: Negative Supply Voltage Input.
> > > +
> > > +  iovcc-supply:
> > > +    description: Digital Input/Output Supply Voltage.
> > > +
> > > +  ref-supply:
> > > +    description:
> > > +      Reference Input/Output. The voltage at the REF pin sets the full-scale
> > > +      range of all channels. If not provided the internal reference is used and
> > > +      also provided on the VREF pin.
> > > +
> > > +  reset-gpios:
> > > +    description:
> > > +      Active-low Asynchronous Clear Input. A logic low at this level-triggered
> > > +      input clears the part to the reset code and range determined by the
> > > +      hardwired option chosen using the MSPAN pins. The control registers are
> > > +      cleared to zero.
> > > +    maxItems: 1
> > > +
> > > +  adi,manual-span-operation-config:
> > > +    description:
> > > +      This property must mimic the MSPAN pin configurations. By tying the
> > MSPAN
> > > +      pins (MSP2, MSP1 and MSP0) to GND and/or VCC, any output range can
> > be
> > > +      hardware-configured with different mid-scale or zero-scale reset options.
> > > +      The hardware configuration is latched during power on reset for proper
> > > +      operation.
> > > +        0 - MPS2=GND, MPS1=GND, MSP0=GND (+-10V, reset to 0V)
> > > +        1 - MPS2=GND, MPS1=GND, MSP0=VCC (+-5V, reset to 0V)
> > > +        2 - MPS2=GND, MPS1=VCC, MSP0=GND (+-2.5V, reset to 0V)
> > > +        3 - MPS2=GND, MPS1=VCC, MSP0=VCC (0V to 10, reset to 0V)
> > > +        4 - MPS2=VCC, MPS1=GND, MSP0=GND (0V to 10V, reset to 5V)
> > > +        5 - MPS2=VCC, MPS1=GND, MSP0=VCC (0V to 5V, reset to 0V)
> > > +        6 - MPS2=VCC, MPS1=VCC, MSP0=GND (0V to 5V, reset to 2.5V)
> > > +        7 - MPS2=VCC, MPS1=VCC, MSP0=VCC (0V to 5V, reset to 0V, enables
> > SoftSpan)
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    enum: [0, 1, 2, 3, 4, 5, 6, 7]
> > 
> > Can you explain why this property is required, when below there's one that sets
> > the ranges in microvolts? Isn't the only new information that this provides the
> > reset values (in a few cases that it is not 0).
> > What am I missing?
> 
> For specifying output range and reset options without relying on software initialization
> routines, and also for enabling the softspan feature, I think this property seems essential.

The portion of this information that is duplicated below I do not think
should reappear in this property.
Really the only additional information is the reset values being at 50%
of the range and the SoftSpan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v4 3/5] dt-bindings: iio: dac: Add adi,ltc2664.yaml
  2024-06-24 15:13     ` Paller, Kim Seer
  2024-06-24 16:13       ` Conor Dooley
@ 2024-06-24 16:48       ` David Lechner
  2024-06-25 15:41         ` Paller, Kim Seer
  1 sibling, 1 reply; 24+ messages in thread
From: David Lechner @ 2024-06-24 16:48 UTC (permalink / raw)
  To: Paller, Kim Seer, Conor Dooley
  Cc: linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org,
	devicetree@vger.kernel.org, Jonathan Cameron, Lars-Peter Clausen,
	Liam Girdwood, Mark Brown, Dimitri Fedrau, Krzysztof Kozlowski,
	Rob Herring, Conor Dooley, Hennerich, Michael, Nuno Sá


>>> +  adi,manual-span-operation-config:
>>> +    description:
>>> +      This property must mimic the MSPAN pin configurations. By tying the
>> MSPAN
>>> +      pins (MSP2, MSP1 and MSP0) to GND and/or VCC, any output range can
>> be
>>> +      hardware-configured with different mid-scale or zero-scale reset options.
>>> +      The hardware configuration is latched during power on reset for proper
>>> +      operation.
>>> +        0 - MPS2=GND, MPS1=GND, MSP0=GND (+-10V, reset to 0V)
>>> +        1 - MPS2=GND, MPS1=GND, MSP0=VCC (+-5V, reset to 0V)
>>> +        2 - MPS2=GND, MPS1=VCC, MSP0=GND (+-2.5V, reset to 0V)
>>> +        3 - MPS2=GND, MPS1=VCC, MSP0=VCC (0V to 10, reset to 0V)
>>> +        4 - MPS2=VCC, MPS1=GND, MSP0=GND (0V to 10V, reset to 5V)
>>> +        5 - MPS2=VCC, MPS1=GND, MSP0=VCC (0V to 5V, reset to 0V)
>>> +        6 - MPS2=VCC, MPS1=VCC, MSP0=GND (0V to 5V, reset to 2.5V)
>>> +        7 - MPS2=VCC, MPS1=VCC, MSP0=VCC (0V to 5V, reset to 0V, enables
>> SoftSpan)
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    enum: [0, 1, 2, 3, 4, 5, 6, 7]
>>
>> Can you explain why this property is required, when below there's one that sets
>> the ranges in microvolts? Isn't the only new information that this provides the
>> reset values (in a few cases that it is not 0).
>> What am I missing?
> 
> For specifying output range and reset options without relying on software initialization
> routines, and also for enabling the softspan feature, I think this property seems essential.

So in other words, this describes how the MSP pins are hardwired and
the per-channel adi,output-range-microvolt is only permissible if

	 adi,manual-span-operation-config = <7>;

(or omitted since 7 is the default)

because in that case each individual pin could have a different
required range based on what is wire up to it?

But if adi,manual-span-operation-config is anything other than 7,
then adi,output-range-microvolt should be not allowed since all
channels will have the same range because of the hard-wired pins.

correct?

The description could probably just be simplified to say that
this describes how the 3 pins are hardwired and to see Table 4
in the datasheet to understand the actual implications rather
than reproducing that table here.

But I do agree that we need both properties. I think we are
just missing:

- if:
    properties:
      adi,manual-span-operation-config:
        const: 7
  then:
    patternProperties:
      "^channel@[0-3]$":
       adi,output-range-microvolt: false

(not tested - may need two ifs, one with

- if:
    required:
      - adi,manual-span-operation-config
    properties:
      adi,manual-span-operation-config:
        const: 7

and one with

- if:
    not:
      required:
        - adi,manual-span-operation-config

to make it work properly)

> 
>>> +    default: 7
>>> +
>>> +  io-channels:
>>> +    description:
>>> +      ADC channel to monitor voltages and temperature at the MUXOUT pin.
>>> +    maxItems: 1
>>> +
>>> +  '#address-cells':
>>> +    const: 1
>>> +
>>> +  '#size-cells':
>>> +    const: 0
>>> +
>>> +patternProperties:
>>> +  "^channel@[0-3]$":
>>> +    type: object
>>> +    additionalProperties: false
>>> +
>>> +    properties:
>>> +      reg:
>>> +        description: The channel number representing the DAC output channel.
>>> +        maximum: 3
>>> +
>>> +      adi,toggle-mode:
>>> +        description:
>>> +          Set the channel as a toggle enabled channel. Toggle operation enables
>>> +          fast switching of a DAC output between two different DAC codes
>> without
>>> +          any SPI transaction.
>>> +        type: boolean
>>> +
>>> +      adi,output-range-microvolt:
>>> +        description: Specify the channel output full scale range.
>>> +        oneOf:
>>> +          - items:
>>> +              - const: 0
>>> +              - enum: [5000000, 10000000]
>>> +          - items:
>>> +              - const: -5000000
>>> +              - const: 5000000
>>> +          - items:
>>> +              - const: -10000000
>>> +              - const: 10000000
>>> +          - items:
>>> +              - const: -2500000
>>> +              - const: 2500000
>>> +
>>> +    required:
>>> +      - reg
>>> +      - adi,output-range-microvolt

And adi,output-range-microvolt should not be required. When SoftSpan
is not available (because MSP != 0x7), then the range is determined
by adi,manual-span-operation-config.

And even when adi,manual-span-operation-config = <7>, there is still
a default range, so adi,output-range-microvolt should still not be
required in that case.


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v4 4/5] dt-bindings: iio: dac: Add adi,ltc2672.yaml
  2024-06-24 15:26       ` Paller, Kim Seer
@ 2024-06-24 17:00         ` Conor Dooley
  2024-06-24 17:37           ` Jonathan Cameron
  0 siblings, 1 reply; 24+ messages in thread
From: Conor Dooley @ 2024-06-24 17:00 UTC (permalink / raw)
  To: Paller, Kim Seer
  Cc: Jonathan Cameron, linux-kernel@vger.kernel.org,
	linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	David Lechner, Lars-Peter Clausen, Liam Girdwood, Mark Brown,
	Dimitri Fedrau, Krzysztof Kozlowski, Rob Herring, Conor Dooley,
	Hennerich, Michael, Nuno Sá

[-- Attachment #1: Type: text/plain, Size: 2710 bytes --]

On Mon, Jun 24, 2024 at 03:26:26PM +0000, Paller, Kim Seer wrote:
> 
> 
> > -----Original Message-----
> > From: Jonathan Cameron <jic23@kernel.org>
> > Sent: Sunday, June 23, 2024 9:44 PM
> > To: Conor Dooley <conor@kernel.org>
> > Cc: Paller, Kim Seer <KimSeer.Paller@analog.com>; linux-
> > kernel@vger.kernel.org; linux-iio@vger.kernel.org; devicetree@vger.kernel.org;
> > David Lechner <dlechner@baylibre.com>; Lars-Peter Clausen
> > <lars@metafoo.de>; Liam Girdwood <lgirdwood@gmail.com>; Mark Brown
> > <broonie@kernel.org>; Dimitri Fedrau <dima.fedrau@gmail.com>; Krzysztof
> > Kozlowski <krzk+dt@kernel.org>; Rob Herring <robh@kernel.org>; Conor
> > Dooley <conor+dt@kernel.org>; Hennerich, Michael
> > <Michael.Hennerich@analog.com>; Nuno Sá <noname.nuno@gmail.com>
> > Subject: Re: [PATCH v4 4/5] dt-bindings: iio: dac: Add adi,ltc2672.yaml
> > 
> > [External]
> > 
> > On Wed, 19 Jun 2024 18:57:59 +0100
> > Conor Dooley <conor@kernel.org> wrote:
> > 
> > > On Wed, Jun 19, 2024 at 02:49:03PM +0800, Kim Seer Paller wrote:
> > > > +patternProperties:
> > > > +  "^channel@[0-4]$":
> > > > +    type: object
> > > > +    additionalProperties: false
> > > > +
> > > > +    properties:
> > > > +      reg:
> > > > +        description: The channel number representing the DAC output
> > channel.
> > > > +        maximum: 4
> > > > +
> > > > +      adi,toggle-mode:
> > > > +        description:
> > > > +          Set the channel as a toggle enabled channel. Toggle operation
> > enables
> > > > +          fast switching of a DAC output between two different DAC codes
> > without
> > > > +          any SPI transaction.
> > > > +        type: boolean
> > > > +
> > > > +      adi,output-range-microamp:
> > > > +        description: Specify the channel output full scale range.
> > > > +        enum: [3125000, 6250000, 12500000, 25000000, 50000000,
> > 100000000,
> > > > +               200000000, 300000000]
> > >
> > > IIO folks, is this sort of thing common/likely to exist on other DACs?
> > 
> > Fair point. It is probably time to conclude this is at least moderately common
> > and generalize it - which will need a dac.yaml similar to the one we have for
> > ADCs in adc/adc.yaml.  That will need to make this a per channel node property
> > (same as the adc ones).
> 
> Should I proceed with generalizing common DAC properties in this series and does

I think so, yes.

> this mean somehow removing these common properties from existing DAC bindings?

I think that that one is up to Jonathan.

> > I'd also expect it to always take 2 values. In many cases the first will be 0 but
> > that is fine.
> > 
> > Jonathan
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v4 4/5] dt-bindings: iio: dac: Add adi,ltc2672.yaml
  2024-06-24 17:00         ` Conor Dooley
@ 2024-06-24 17:37           ` Jonathan Cameron
  2024-06-25 15:51             ` Paller, Kim Seer
  0 siblings, 1 reply; 24+ messages in thread
From: Jonathan Cameron @ 2024-06-24 17:37 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Paller, Kim Seer, Jonathan Cameron, linux-kernel@vger.kernel.org,
	linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	David Lechner, Lars-Peter Clausen, Liam Girdwood, Mark Brown,
	Dimitri Fedrau, Krzysztof Kozlowski, Rob Herring, Conor Dooley,
	Hennerich, Michael, Nuno Sá

On Mon, 24 Jun 2024 18:00:24 +0100
Conor Dooley <conor@kernel.org> wrote:

> On Mon, Jun 24, 2024 at 03:26:26PM +0000, Paller, Kim Seer wrote:
> > 
> >   
> > > -----Original Message-----
> > > From: Jonathan Cameron <jic23@kernel.org>
> > > Sent: Sunday, June 23, 2024 9:44 PM
> > > To: Conor Dooley <conor@kernel.org>
> > > Cc: Paller, Kim Seer <KimSeer.Paller@analog.com>; linux-
> > > kernel@vger.kernel.org; linux-iio@vger.kernel.org; devicetree@vger.kernel.org;
> > > David Lechner <dlechner@baylibre.com>; Lars-Peter Clausen
> > > <lars@metafoo.de>; Liam Girdwood <lgirdwood@gmail.com>; Mark Brown
> > > <broonie@kernel.org>; Dimitri Fedrau <dima.fedrau@gmail.com>; Krzysztof
> > > Kozlowski <krzk+dt@kernel.org>; Rob Herring <robh@kernel.org>; Conor
> > > Dooley <conor+dt@kernel.org>; Hennerich, Michael
> > > <Michael.Hennerich@analog.com>; Nuno Sá <noname.nuno@gmail.com>
> > > Subject: Re: [PATCH v4 4/5] dt-bindings: iio: dac: Add adi,ltc2672.yaml
> > > 
> > > [External]
> > > 
> > > On Wed, 19 Jun 2024 18:57:59 +0100
> > > Conor Dooley <conor@kernel.org> wrote:
> > >   
> > > > On Wed, Jun 19, 2024 at 02:49:03PM +0800, Kim Seer Paller wrote:  
> > > > > +patternProperties:
> > > > > +  "^channel@[0-4]$":
> > > > > +    type: object
> > > > > +    additionalProperties: false
> > > > > +
> > > > > +    properties:
> > > > > +      reg:
> > > > > +        description: The channel number representing the DAC output  
> > > channel.  
> > > > > +        maximum: 4
> > > > > +
> > > > > +      adi,toggle-mode:
> > > > > +        description:
> > > > > +          Set the channel as a toggle enabled channel. Toggle operation  
> > > enables  
> > > > > +          fast switching of a DAC output between two different DAC codes  
> > > without  
> > > > > +          any SPI transaction.
> > > > > +        type: boolean
> > > > > +
> > > > > +      adi,output-range-microamp:
> > > > > +        description: Specify the channel output full scale range.
> > > > > +        enum: [3125000, 6250000, 12500000, 25000000, 50000000,  
> > > 100000000,  
> > > > > +               200000000, 300000000]  
> > > >
> > > > IIO folks, is this sort of thing common/likely to exist on other DACs?  
> > > 
> > > Fair point. It is probably time to conclude this is at least moderately common
> > > and generalize it - which will need a dac.yaml similar to the one we have for
> > > ADCs in adc/adc.yaml.  That will need to make this a per channel node property
> > > (same as the adc ones).  
> > 
> > Should I proceed with generalizing common DAC properties in this series and does  
> 
> I think so, yes.

Yes, that would great.

> 
> > this mean somehow removing these common properties from existing DAC bindings?  
> 
> I think that that one is up to Jonathan.

We can deprecate them.  At somepoint we can remove them form the documented bindings
but we will need to keep them in drivers forever (which won't be costly in this case).

Jonathan

> 
> > > I'd also expect it to always take 2 values. In many cases the first will be 0 but
> > > that is fine.
> > > 
> > > Jonathan  
> >   
> 


^ permalink raw reply	[flat|nested] 24+ messages in thread

* RE: [PATCH v4 3/5] dt-bindings: iio: dac: Add adi,ltc2664.yaml
  2024-06-24 16:48       ` David Lechner
@ 2024-06-25 15:41         ` Paller, Kim Seer
  0 siblings, 0 replies; 24+ messages in thread
From: Paller, Kim Seer @ 2024-06-25 15:41 UTC (permalink / raw)
  To: David Lechner, Conor Dooley
  Cc: linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org,
	devicetree@vger.kernel.org, Jonathan Cameron, Lars-Peter Clausen,
	Liam Girdwood, Mark Brown, Dimitri Fedrau, Krzysztof Kozlowski,
	Rob Herring, Conor Dooley, Hennerich, Michael, Nuno Sá



> -----Original Message-----
> From: David Lechner <dlechner@baylibre.com>
> Sent: Tuesday, June 25, 2024 12:49 AM
> To: Paller, Kim Seer <KimSeer.Paller@analog.com>; Conor Dooley
> <conor@kernel.org>
> Cc: linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org;
> devicetree@vger.kernel.org; Jonathan Cameron <jic23@kernel.org>; Lars-Peter
> Clausen <lars@metafoo.de>; Liam Girdwood <lgirdwood@gmail.com>; Mark
> Brown <broonie@kernel.org>; Dimitri Fedrau <dima.fedrau@gmail.com>;
> Krzysztof Kozlowski <krzk+dt@kernel.org>; Rob Herring <robh@kernel.org>;
> Conor Dooley <conor+dt@kernel.org>; Hennerich, Michael
> <Michael.Hennerich@analog.com>; Nuno Sá <noname.nuno@gmail.com>
> Subject: Re: [PATCH v4 3/5] dt-bindings: iio: dac: Add adi,ltc2664.yaml
> 
> [External]
> 
> 
> >>> +  adi,manual-span-operation-config:
> >>> +    description:
> >>> +      This property must mimic the MSPAN pin configurations. By tying the
> >> MSPAN
> >>> +      pins (MSP2, MSP1 and MSP0) to GND and/or VCC, any output range
> can
> >> be
> >>> +      hardware-configured with different mid-scale or zero-scale reset
> options.
> >>> +      The hardware configuration is latched during power on reset for
> proper
> >>> +      operation.
> >>> +        0 - MPS2=GND, MPS1=GND, MSP0=GND (+-10V, reset to 0V)
> >>> +        1 - MPS2=GND, MPS1=GND, MSP0=VCC (+-5V, reset to 0V)
> >>> +        2 - MPS2=GND, MPS1=VCC, MSP0=GND (+-2.5V, reset to 0V)
> >>> +        3 - MPS2=GND, MPS1=VCC, MSP0=VCC (0V to 10, reset to 0V)
> >>> +        4 - MPS2=VCC, MPS1=GND, MSP0=GND (0V to 10V, reset to 5V)
> >>> +        5 - MPS2=VCC, MPS1=GND, MSP0=VCC (0V to 5V, reset to 0V)
> >>> +        6 - MPS2=VCC, MPS1=VCC, MSP0=GND (0V to 5V, reset to 2.5V)
> >>> +        7 - MPS2=VCC, MPS1=VCC, MSP0=VCC (0V to 5V, reset to 0V, enables
> >> SoftSpan)
> >>> +    $ref: /schemas/types.yaml#/definitions/uint32
> >>> +    enum: [0, 1, 2, 3, 4, 5, 6, 7]
> >>
> >> Can you explain why this property is required, when below there's one that
> sets
> >> the ranges in microvolts? Isn't the only new information that this provides
> the
> >> reset values (in a few cases that it is not 0).
> >> What am I missing?
> >
> > For specifying output range and reset options without relying on software
> initialization
> > routines, and also for enabling the softspan feature, I think this property
> seems essential.
> 
> So in other words, this describes how the MSP pins are hardwired and
> the per-channel adi,output-range-microvolt is only permissible if
> 
> 	 adi,manual-span-operation-config = <7>;
> 
> (or omitted since 7 is the default)
> 
> because in that case each individual pin could have a different
> required range based on what is wire up to it?
> 
> But if adi,manual-span-operation-config is anything other than 7,
> then adi,output-range-microvolt should be not allowed since all
> channels will have the same range because of the hard-wired pins.
> 
> correct?

That's correct.
 
> The description could probably just be simplified to say that
> this describes how the 3 pins are hardwired and to see Table 4
> in the datasheet to understand the actual implications rather
> than reproducing that table here.
> 
> But I do agree that we need both properties. I think we are
> just missing:
> 
> - if:
>     properties:
>       adi,manual-span-operation-config:
>         const: 7
>   then:
>     patternProperties:
>       "^channel@[0-3]$":
>        adi,output-range-microvolt: false
> 
> (not tested - may need two ifs, one with
> 
> - if:
>     required:
>       - adi,manual-span-operation-config
>     properties:
>       adi,manual-span-operation-config:
>         const: 7
> 
> and one with
> 
> - if:
>     not:
>       required:
>         - adi,manual-span-operation-config
> 
> to make it work properly)
> 
> >
> >>> +    default: 7
> >>> +
> >>> +  io-channels:
> >>> +    description:
> >>> +      ADC channel to monitor voltages and temperature at the MUXOUT
> pin.
> >>> +    maxItems: 1
> >>> +
> >>> +  '#address-cells':
> >>> +    const: 1
> >>> +
> >>> +  '#size-cells':
> >>> +    const: 0
> >>> +
> >>> +patternProperties:
> >>> +  "^channel@[0-3]$":
> >>> +    type: object
> >>> +    additionalProperties: false
> >>> +
> >>> +    properties:
> >>> +      reg:
> >>> +        description: The channel number representing the DAC output
> channel.
> >>> +        maximum: 3
> >>> +
> >>> +      adi,toggle-mode:
> >>> +        description:
> >>> +          Set the channel as a toggle enabled channel. Toggle operation
> enables
> >>> +          fast switching of a DAC output between two different DAC codes
> >> without
> >>> +          any SPI transaction.
> >>> +        type: boolean
> >>> +
> >>> +      adi,output-range-microvolt:
> >>> +        description: Specify the channel output full scale range.
> >>> +        oneOf:
> >>> +          - items:
> >>> +              - const: 0
> >>> +              - enum: [5000000, 10000000]
> >>> +          - items:
> >>> +              - const: -5000000
> >>> +              - const: 5000000
> >>> +          - items:
> >>> +              - const: -10000000
> >>> +              - const: 10000000
> >>> +          - items:
> >>> +              - const: -2500000
> >>> +              - const: 2500000
> >>> +
> >>> +    required:
> >>> +      - reg
> >>> +      - adi,output-range-microvolt
> 
> And adi,output-range-microvolt should not be required. When SoftSpan
> is not available (because MSP != 0x7), then the range is determined
> by adi,manual-span-operation-config.
> 
> And even when adi,manual-span-operation-config = <7>, there is still
> a default range, so adi,output-range-microvolt should still not be
> required in that case.

This makes sense how this operates. Appreciate the detailed feedback.


^ permalink raw reply	[flat|nested] 24+ messages in thread

* RE: [PATCH v4 4/5] dt-bindings: iio: dac: Add adi,ltc2672.yaml
  2024-06-24 17:37           ` Jonathan Cameron
@ 2024-06-25 15:51             ` Paller, Kim Seer
  2024-06-25 16:20               ` Conor Dooley
  0 siblings, 1 reply; 24+ messages in thread
From: Paller, Kim Seer @ 2024-06-25 15:51 UTC (permalink / raw)
  To: Jonathan Cameron, Conor Dooley
  Cc: Jonathan Cameron, linux-kernel@vger.kernel.org,
	linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	David Lechner, Lars-Peter Clausen, Liam Girdwood, Mark Brown,
	Dimitri Fedrau, Krzysztof Kozlowski, Rob Herring, Conor Dooley,
	Hennerich, Michael, Nuno Sá



> -----Original Message-----
> From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
> Sent: Tuesday, June 25, 2024 1:38 AM
> To: Conor Dooley <conor@kernel.org>
> Cc: Paller, Kim Seer <KimSeer.Paller@analog.com>; Jonathan Cameron
> <jic23@kernel.org>; linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org;
> devicetree@vger.kernel.org; David Lechner <dlechner@baylibre.com>; Lars-
> Peter Clausen <lars@metafoo.de>; Liam Girdwood <lgirdwood@gmail.com>;
> Mark Brown <broonie@kernel.org>; Dimitri Fedrau <dima.fedrau@gmail.com>;
> Krzysztof Kozlowski <krzk+dt@kernel.org>; Rob Herring <robh@kernel.org>;
> Conor Dooley <conor+dt@kernel.org>; Hennerich, Michael
> <Michael.Hennerich@analog.com>; Nuno Sá <noname.nuno@gmail.com>
> Subject: Re: [PATCH v4 4/5] dt-bindings: iio: dac: Add adi,ltc2672.yaml
> 
> [External]
> 
> On Mon, 24 Jun 2024 18:00:24 +0100
> Conor Dooley <conor@kernel.org> wrote:
> 
> > On Mon, Jun 24, 2024 at 03:26:26PM +0000, Paller, Kim Seer wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Jonathan Cameron <jic23@kernel.org>
> > > > Sent: Sunday, June 23, 2024 9:44 PM
> > > > To: Conor Dooley <conor@kernel.org>
> > > > Cc: Paller, Kim Seer <KimSeer.Paller@analog.com>; linux-
> > > > kernel@vger.kernel.org; linux-iio@vger.kernel.org;
> devicetree@vger.kernel.org;
> > > > David Lechner <dlechner@baylibre.com>; Lars-Peter Clausen
> > > > <lars@metafoo.de>; Liam Girdwood <lgirdwood@gmail.com>; Mark
> Brown
> > > > <broonie@kernel.org>; Dimitri Fedrau <dima.fedrau@gmail.com>;
> Krzysztof
> > > > Kozlowski <krzk+dt@kernel.org>; Rob Herring <robh@kernel.org>; Conor
> > > > Dooley <conor+dt@kernel.org>; Hennerich, Michael
> > > > <Michael.Hennerich@analog.com>; Nuno Sá <noname.nuno@gmail.com>
> > > > Subject: Re: [PATCH v4 4/5] dt-bindings: iio: dac: Add adi,ltc2672.yaml
> > > >
> > > > [External]
> > > >
> > > > On Wed, 19 Jun 2024 18:57:59 +0100
> > > > Conor Dooley <conor@kernel.org> wrote:
> > > >
> > > > > On Wed, Jun 19, 2024 at 02:49:03PM +0800, Kim Seer Paller wrote:
> > > > > > +patternProperties:
> > > > > > +  "^channel@[0-4]$":
> > > > > > +    type: object
> > > > > > +    additionalProperties: false
> > > > > > +
> > > > > > +    properties:
> > > > > > +      reg:
> > > > > > +        description: The channel number representing the DAC output
> > > > channel.
> > > > > > +        maximum: 4
> > > > > > +
> > > > > > +      adi,toggle-mode:
> > > > > > +        description:
> > > > > > +          Set the channel as a toggle enabled channel. Toggle operation
> > > > enables
> > > > > > +          fast switching of a DAC output between two different DAC
> codes
> > > > without
> > > > > > +          any SPI transaction.
> > > > > > +        type: boolean
> > > > > > +
> > > > > > +      adi,output-range-microamp:
> > > > > > +        description: Specify the channel output full scale range.
> > > > > > +        enum: [3125000, 6250000, 12500000, 25000000, 50000000,
> > > > 100000000,
> > > > > > +               200000000, 300000000]
> > > > >
> > > > > IIO folks, is this sort of thing common/likely to exist on other DACs?
> > > >
> > > > Fair point. It is probably time to conclude this is at least moderately
> common
> > > > and generalize it - which will need a dac.yaml similar to the one we have
> for
> > > > ADCs in adc/adc.yaml.  That will need to make this a per channel node
> property
> > > > (same as the adc ones).
> > >
> > > Should I proceed with generalizing common DAC properties in this series
> and does
> >
> > I think so, yes.
> 
> Yes, that would great.

I was wondering who would be the designated maintainer for new dac.yaml?

> >
> > > this mean somehow removing these common properties from existing DAC
> bindings?
> >
> > I think that that one is up to Jonathan.
> 
> We can deprecate them.  At somepoint we can remove them form the
> documented bindings
> but we will need to keep them in drivers forever (which won't be costly in this
> case).
> 
> Jonathan
> 
> >
> > > > I'd also expect it to always take 2 values. In many cases the first will be 0
> but
> > > > that is fine.
> > > >
> > > > Jonathan
> > >
> >


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v4 4/5] dt-bindings: iio: dac: Add adi,ltc2672.yaml
  2024-06-25 15:51             ` Paller, Kim Seer
@ 2024-06-25 16:20               ` Conor Dooley
  2024-06-28 19:02                 ` Jonathan Cameron
  0 siblings, 1 reply; 24+ messages in thread
From: Conor Dooley @ 2024-06-25 16:20 UTC (permalink / raw)
  To: Paller, Kim Seer
  Cc: Jonathan Cameron, Jonathan Cameron, linux-kernel@vger.kernel.org,
	linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	David Lechner, Lars-Peter Clausen, Liam Girdwood, Mark Brown,
	Dimitri Fedrau, Krzysztof Kozlowski, Rob Herring, Conor Dooley,
	Hennerich, Michael, Nuno Sá

[-- Attachment #1: Type: text/plain, Size: 3053 bytes --]

On Tue, Jun 25, 2024 at 03:51:27PM +0000, Paller, Kim Seer wrote:
> 
> 
> > From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>

> > On Mon, 24 Jun 2024 18:00:24 +0100
> > Conor Dooley <conor@kernel.org> wrote:
> > 
> > > On Mon, Jun 24, 2024 at 03:26:26PM +0000, Paller, Kim Seer wrote:
> > > >
> > > >
> > > > > From: Jonathan Cameron <jic23@kernel.org>

> > > > > Subject: Re: [PATCH v4 4/5] dt-bindings: iio: dac: Add adi,ltc2672.yaml
> > > > >
> > > > >
> > > > > On Wed, 19 Jun 2024 18:57:59 +0100
> > > > > Conor Dooley <conor@kernel.org> wrote:
> > > > >
> > > > > > On Wed, Jun 19, 2024 at 02:49:03PM +0800, Kim Seer Paller wrote:
> > > > > > > +patternProperties:
> > > > > > > +  "^channel@[0-4]$":
> > > > > > > +    type: object
> > > > > > > +    additionalProperties: false
> > > > > > > +
> > > > > > > +    properties:
> > > > > > > +      reg:
> > > > > > > +        description: The channel number representing the DAC output
> > > > > channel.
> > > > > > > +        maximum: 4
> > > > > > > +
> > > > > > > +      adi,toggle-mode:
> > > > > > > +        description:
> > > > > > > +          Set the channel as a toggle enabled channel. Toggle operation
> > > > > enables
> > > > > > > +          fast switching of a DAC output between two different DAC
> > codes
> > > > > without
> > > > > > > +          any SPI transaction.
> > > > > > > +        type: boolean
> > > > > > > +
> > > > > > > +      adi,output-range-microamp:
> > > > > > > +        description: Specify the channel output full scale range.
> > > > > > > +        enum: [3125000, 6250000, 12500000, 25000000, 50000000,
> > > > > 100000000,
> > > > > > > +               200000000, 300000000]
> > > > > >
> > > > > > IIO folks, is this sort of thing common/likely to exist on other DACs?
> > > > >
> > > > > Fair point. It is probably time to conclude this is at least moderately
> > common
> > > > > and generalize it - which will need a dac.yaml similar to the one we have
> > for
> > > > > ADCs in adc/adc.yaml.  That will need to make this a per channel node
> > property
> > > > > (same as the adc ones).
> > > >
> > > > Should I proceed with generalizing common DAC properties in this series
> > and does
> > >
> > > I think so, yes.
> > 
> > Yes, that would great.
> 
> I was wondering who would be the designated maintainer for new dac.yaml?

I'd suggest Jonathan!

> > > > this mean somehow removing these common properties from existing DAC
> > bindings?
> > >
> > > I think that that one is up to Jonathan.
> > 
> > We can deprecate them.  At somepoint we can remove them form the
> > documented bindings
> > but we will need to keep them in drivers forever (which won't be costly in this
> > case).

Right. Anything where the name _would change_ needs to be deprecated and
kept forever. I was thinking more about properties that are defined in
multiple locations with the same name, e.g. if "output-range-microvolts"
existed in 6 different bindings.

Thanks,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v4 5/5] iio: dac: ltc2664: Add driver for LTC2664 and LTC2672
  2024-06-19  6:49 ` [PATCH v4 5/5] iio: dac: ltc2664: Add driver for LTC2664 and LTC2672 Kim Seer Paller
  2024-06-20  0:00   ` Paller, Kim Seer
  2024-06-23 13:50   ` Jonathan Cameron
@ 2024-06-26 11:14   ` Nuno Sá
  2 siblings, 0 replies; 24+ messages in thread
From: Nuno Sá @ 2024-06-26 11:14 UTC (permalink / raw)
  To: Kim Seer Paller, linux-kernel, linux-iio, devicetree
  Cc: Jonathan Cameron, David Lechner, Lars-Peter Clausen,
	Liam Girdwood, Mark Brown, Dimitri Fedrau, Krzysztof Kozlowski,
	Rob Herring, Conor Dooley, Michael Hennerich

On Wed, 2024-06-19 at 14:49 +0800, Kim Seer Paller wrote:
> LTC2664 4 channel, 12-/16-Bit Voltage Output SoftSpan DAC
> LTC2672 5 channel, 12-/16-Bit Current Output Softspan DAC
> 
> Co-developed-by: Michael Hennerich <michael.hennerich@analog.com>
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> ---

Just minor nits... Anyways:

Reviewed-by: Nuno Sa <nuno.sa@analog.com>

>  MAINTAINERS               |   1 +
>  drivers/iio/dac/Kconfig   |  11 +
>  drivers/iio/dac/Makefile  |   1 +
>  drivers/iio/dac/ltc2664.c | 755 ++++++++++++++++++++++++++++++++++++++
>  4 files changed, 768 insertions(+)
>  create mode 100644 drivers/iio/dac/ltc2664.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f4a5b5bc8ccc..7a02d9a196fb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13082,6 +13082,7 @@ S:	Supported
>  W:	https://ez.analog.com/linux-software-drivers
>  F:	Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
>  F:	Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml
> +F:	drivers/iio/dac/ltc2664.c
>  
>  LTC2688 IIO DAC DRIVER
>  M:	Nuno Sá <nuno.sa@analog.com>
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index fb48dddbcc20..3a7691db3998 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -371,6 +371,17 @@ config LTC2632
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called ltc2632.
>  
> +config LTC2664
> +	tristate "Analog Devices LTC2664 and LTC2672 DAC SPI driver"
> +	depends on SPI
> +	select REGMAP
> +	help
> +	  Say yes here to build support for Analog Devices
> +	  LTC2664 and LTC2672 converters (DAC).
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ltc2664.
> +
>  config M62332
>  	tristate "Mitsubishi M62332 DAC driver"
>  	depends on I2C
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index 8432a81a19dc..2cf148f16306 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -37,6 +37,7 @@ obj-$(CONFIG_DS4424) += ds4424.o
>  obj-$(CONFIG_LPC18XX_DAC) += lpc18xx_dac.o
>  obj-$(CONFIG_LTC1660) += ltc1660.o
>  obj-$(CONFIG_LTC2632) += ltc2632.o
> +obj-$(CONFIG_LTC2664) += ltc2664.o
>  obj-$(CONFIG_LTC2688) += ltc2688.o
>  obj-$(CONFIG_M62332) += m62332.o
>  obj-$(CONFIG_MAX517) += max517.o
> diff --git a/drivers/iio/dac/ltc2664.c b/drivers/iio/dac/ltc2664.c
> new file mode 100644
> index 000000000000..9b73b9c6a7a7
> --- /dev/null
> +++ b/drivers/iio/dac/ltc2664.c
> @@ -0,0 +1,755 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * LTC2664 4 channel, 12-/16-Bit Voltage Output SoftSpan DAC driver
> + * LTC2672 5 channel, 12-/16-Bit Current Output Softspan DAC driver
> + *
> + * Copyright 2024 Analog Devices Inc.
> + */
> +
> 

...

> +
> +static int ltc2664_dac_code_read(struct ltc2664_state *st, u32 chan, u32
> input,
> +				 u32 *code)
> +{
> +	guard(mutex)(&st->lock);
> +	*code = st->channels[chan].raw[input];
> +
> +	return 0;

no need for an error code...

...

> 
> +static int ltc2664_probe(struct spi_device *spi)
> +{
> +	static const char * const regulators[] = { "vcc", "iovcc", "v-neg" };
> +	const struct ltc2664_chip_info *chip_info;
> +	struct device *dev = &spi->dev;
> +	struct iio_dev *indio_dev;
> +	struct ltc2664_state *st;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	st->spi = spi;
> +
> +	chip_info = spi_get_device_match_data(spi);
> +	if (!chip_info)
> +		return -ENODEV;
> +
> +	st->chip_info = chip_info;
> +
> +	mutex_init(&st->lock);
> +
> +	st->regmap = devm_regmap_init_spi(spi, &ltc2664_regmap_config);
> +	if (IS_ERR(st->regmap))
> +		return dev_err_probe(dev, PTR_ERR(st->regmap),
> +				     "Failed to init regmap");
> +
> +	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulators),
> +					     regulators);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to enable
> regulators\n");
> +
> +	ret = devm_regulator_get_enable_read_voltage(dev, "ref");
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Failed to get vref
> voltage\n");
> +	else if (ret == -ENODEV)
> +		st->vref_mv = chip_info->internal_vref_mv;
> +	else
> +		st->vref_mv = ret / 1000;

This could be:
if (ret < 0 && ret != -ENODEV)
	return ret;

st->vref_mv = ret > 0 ? ret / 1000 :  chip_info->internal_vref_mv;

- Nuno Sá



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v4 4/5] dt-bindings: iio: dac: Add adi,ltc2672.yaml
  2024-06-25 16:20               ` Conor Dooley
@ 2024-06-28 19:02                 ` Jonathan Cameron
  0 siblings, 0 replies; 24+ messages in thread
From: Jonathan Cameron @ 2024-06-28 19:02 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Paller, Kim Seer, Jonathan Cameron, linux-kernel@vger.kernel.org,
	linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	David Lechner, Lars-Peter Clausen, Liam Girdwood, Mark Brown,
	Dimitri Fedrau, Krzysztof Kozlowski, Rob Herring, Conor Dooley,
	Hennerich, Michael, Nuno Sá

On Tue, 25 Jun 2024 17:20:48 +0100
Conor Dooley <conor@kernel.org> wrote:

> On Tue, Jun 25, 2024 at 03:51:27PM +0000, Paller, Kim Seer wrote:
> > 
> >   
> > > From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>  
> 
> > > On Mon, 24 Jun 2024 18:00:24 +0100
> > > Conor Dooley <conor@kernel.org> wrote:
> > >   
> > > > On Mon, Jun 24, 2024 at 03:26:26PM +0000, Paller, Kim Seer wrote:  
> > > > >
> > > > >  
> > > > > > From: Jonathan Cameron <jic23@kernel.org>  
> 
> > > > > > Subject: Re: [PATCH v4 4/5] dt-bindings: iio: dac: Add adi,ltc2672.yaml
> > > > > >
> > > > > >
> > > > > > On Wed, 19 Jun 2024 18:57:59 +0100
> > > > > > Conor Dooley <conor@kernel.org> wrote:
> > > > > >  
> > > > > > > On Wed, Jun 19, 2024 at 02:49:03PM +0800, Kim Seer Paller wrote:  
> > > > > > > > +patternProperties:
> > > > > > > > +  "^channel@[0-4]$":
> > > > > > > > +    type: object
> > > > > > > > +    additionalProperties: false
> > > > > > > > +
> > > > > > > > +    properties:
> > > > > > > > +      reg:
> > > > > > > > +        description: The channel number representing the DAC output  
> > > > > > channel.  
> > > > > > > > +        maximum: 4
> > > > > > > > +
> > > > > > > > +      adi,toggle-mode:
> > > > > > > > +        description:
> > > > > > > > +          Set the channel as a toggle enabled channel. Toggle operation  
> > > > > > enables  
> > > > > > > > +          fast switching of a DAC output between two different DAC  
> > > codes  
> > > > > > without  
> > > > > > > > +          any SPI transaction.
> > > > > > > > +        type: boolean
> > > > > > > > +
> > > > > > > > +      adi,output-range-microamp:
> > > > > > > > +        description: Specify the channel output full scale range.
> > > > > > > > +        enum: [3125000, 6250000, 12500000, 25000000, 50000000,  
> > > > > > 100000000,  
> > > > > > > > +               200000000, 300000000]  
> > > > > > >
> > > > > > > IIO folks, is this sort of thing common/likely to exist on other DACs?  
> > > > > >
> > > > > > Fair point. It is probably time to conclude this is at least moderately  
> > > common  
> > > > > > and generalize it - which will need a dac.yaml similar to the one we have  
> > > for  
> > > > > > ADCs in adc/adc.yaml.  That will need to make this a per channel node  
> > > property  
> > > > > > (same as the adc ones).  
> > > > >
> > > > > Should I proceed with generalizing common DAC properties in this series  
> > > and does  
> > > >
> > > > I think so, yes.  
> > > 
> > > Yes, that would great.  
> > 
> > I was wondering who would be the designated maintainer for new dac.yaml?  
> 
> I'd suggest Jonathan!

Sure.  Though if anyone else wants to maintain this one they'd be welcome :)

> 
> > > > > this mean somehow removing these common properties from existing DAC  
> > > bindings?  
> > > >
> > > > I think that that one is up to Jonathan.  
> > > 
> > > We can deprecate them.  At somepoint we can remove them form the
> > > documented bindings
> > > but we will need to keep them in drivers forever (which won't be costly in this
> > > case).  
> 
> Right. Anything where the name _would change_ needs to be deprecated and
> kept forever. I was thinking more about properties that are defined in
> multiple locations with the same name, e.g. if "output-range-microvolts"
> existed in 6 different bindings.
> 
> Thanks,
> Conor.


^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2024-06-28 19:02 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-19  6:48 [PATCH v4 0/5] Add driver for LTC2664 and LTC2672 Kim Seer Paller
2024-06-19  6:49 ` [PATCH v4 1/5] iio: ABI: Generalize ABI documentation for DAC Kim Seer Paller
2024-06-19  6:49 ` [PATCH v4 2/5] iio: ABI: add DAC 42kohm_to_gnd powerdown mode Kim Seer Paller
2024-06-19  6:49 ` [PATCH v4 3/5] dt-bindings: iio: dac: Add adi,ltc2664.yaml Kim Seer Paller
2024-06-19 17:56   ` Conor Dooley
2024-06-24 15:13     ` Paller, Kim Seer
2024-06-24 16:13       ` Conor Dooley
2024-06-24 16:48       ` David Lechner
2024-06-25 15:41         ` Paller, Kim Seer
2024-06-19  6:49 ` [PATCH v4 4/5] dt-bindings: iio: dac: Add adi,ltc2672.yaml Kim Seer Paller
2024-06-19 17:57   ` Conor Dooley
2024-06-23 13:43     ` Jonathan Cameron
2024-06-23 14:03       ` Conor Dooley
2024-06-23 16:20         ` Jonathan Cameron
2024-06-24 15:26       ` Paller, Kim Seer
2024-06-24 17:00         ` Conor Dooley
2024-06-24 17:37           ` Jonathan Cameron
2024-06-25 15:51             ` Paller, Kim Seer
2024-06-25 16:20               ` Conor Dooley
2024-06-28 19:02                 ` Jonathan Cameron
2024-06-19  6:49 ` [PATCH v4 5/5] iio: dac: ltc2664: Add driver for LTC2664 and LTC2672 Kim Seer Paller
2024-06-20  0:00   ` Paller, Kim Seer
2024-06-23 13:50   ` Jonathan Cameron
2024-06-26 11:14   ` Nuno Sá

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).