* [PATCH 0/2] Adding support for Microchip MCP47FEB02
@ 2025-09-22 11:30 Ariana Lazar
2025-09-22 11:30 ` [PATCH 1/2] dt-bindings: iio: dac: adding " Ariana Lazar
2025-09-22 11:30 ` [PATCH 2/2] " Ariana Lazar
0 siblings, 2 replies; 14+ messages in thread
From: Ariana Lazar @ 2025-09-22 11:30 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-iio, devicetree, linux-kernel, Ariana Lazar
Adding support for Microchip MCP47F(E/V)B(0/1/2)1, MCP47F(E/V)B(0/1/2)2,
MCP47F(E/V)B(0/1/2)4 and MCP47F(E/V)B(0/1/2)8 series of buffered voltage
output Digital-to-Analog converters with an I2C Interface. This driver
covers the following part numbers:
- With nonvolatile memory:
- MCP47FEB01, MCP47FEB11, MCP47FEB21, MCP47FEB02, MCP47FEB12
- MCP47FEB22, MCP47FVB01, MCP47FVB11, MCP47FVB21, MCP47FVB02
- With volatile memory:
- MCP47FVB12, MCP47FVB02, MCP47FVB12, MCP47FVB22, MCP47FVB04
- MCP47FVB14, MCP47FVB24, MCP47FVB04, MCP47FVB08, MCP47FVB18
- MCP47FVB28, MCP47FEB04, MCP47FEB14 and MCP47FEB24
The families support up to 8 output channels. The devices can be 8-bit,
10-bit and 12-bit resolution.
Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com>
---
Ariana Lazar (2):
dt-bindings: iio: dac: adding support for Microchip MCP47FEB02
iio: dac: adding support for Microchip MCP47FEB02
.../bindings/iio/dac/microchip,mcp47feb02.yaml | 305 +++++
MAINTAINERS | 7 +
drivers/iio/dac/Kconfig | 16 +
drivers/iio/dac/Makefile | 1 +
drivers/iio/dac/mcp47feb02.c | 1347 ++++++++++++++++++++
5 files changed, 1676 insertions(+)
---
base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
change-id: 20250825-mcp47feb02-31c77857ae29
Best regards,
--
Ariana Lazar <ariana.lazar@microchip.com>
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 1/2] dt-bindings: iio: dac: adding support for Microchip MCP47FEB02 2025-09-22 11:30 [PATCH 0/2] Adding support for Microchip MCP47FEB02 Ariana Lazar @ 2025-09-22 11:30 ` Ariana Lazar 2025-09-22 16:38 ` Rob Herring (Arm) ` (2 more replies) 2025-09-22 11:30 ` [PATCH 2/2] " Ariana Lazar 1 sibling, 3 replies; 14+ messages in thread From: Ariana Lazar @ 2025-09-22 11:30 UTC (permalink / raw) To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-iio, devicetree, linux-kernel, Ariana Lazar This is the device tree schema for iio driver for Microchip MCP47F(E/V)B(0/1/2)1, MCP47F(E/V)B(0/1/2)2, MCP47F(E/V)B(0/1/2)4 and MCP47F(E/V)B(0/1/2)8 series of buffered voltage output Digital-to-Analog Converters with nonvolatile or volatile memory and an I2C Interface. The families support up to 8 output channels. The devices can be 8-bit, 10-bit and 12-bit. Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com> --- .../bindings/iio/dac/microchip,mcp47feb02.yaml | 305 +++++++++++++++++++++ MAINTAINERS | 6 + 2 files changed, 311 insertions(+) diff --git a/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml b/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml new file mode 100644 index 0000000000000000000000000000000000000000..d05ddafa37540bc1f6b6ce65a466b95913925c10 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml @@ -0,0 +1,305 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/dac/microchip,mcp47feb02.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Microchip MCP47F(E/V)B(0/1/2)(1/2/4/8) DAC with I2C Interface Families + +maintainers: + - Ariana Lazar <ariana.lazar@microchip.com> + +description: | + Datasheet for MCP47FEB01, MCP47FEB11, MCP47FEB21, MCP47FEB02, MCP47FEB12, + MCP47FEB22 can be found here: + https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005375A.pdf + Datasheet for MCP47FVBXX can be found here: + https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005405A.pdf + Datasheet for MCP47FEB04, MCP47FEB14, MCP47FEB24, MCP47FEB08, MCP47FEB18, + MCP47FEB28, MCP47FVB04, MCP47FVB14, MCP47FVB24, MCP47FVB08, MCP47FVB18, + MCP47FVB28 can be found here: + https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/MCP47FXBX48-Data-Sheet-DS200006368A.pdf + + +------------+--------------+-------------+-------------+------------+ + | Device | Resolution | Channels | Vref number | Memory | + |------------|--------------|-------------|-------------|------------| + | MCP47FEB01 | 8-bit | 1 | 1 | EEPROM | + | MCP47FEB11 | 10-bit | 1 | 1 | EEPROM | + | MCP47FEB21 | 12-bit | 1 | 1 | EEPROM | + |------------|--------------|-------------|-------------|------------| + | MCP47FEB02 | 8-bit | 2 | 1 | EEPROM | + | MCP47FEB12 | 10-bit | 2 | 1 | EEPROM | + | MCP47FEB22 | 12-bit | 2 | 1 | EEPROM | + |------------|--------------|-------------|-------------|------------| + | MCP47FVB01 | 8-bit | 1 | 1 | RAM | + | MCP47FVB11 | 10-bit | 1 | 1 | RAM | + | MCP47FVB21 | 12-bit | 1 | 1 | RAM | + |------------|--------------|-------------|-------------|------------| + | MCP47FVB02 | 8-bit | 2 | 1 | RAM | + | MCP47FVB12 | 10-bit | 2 | 1 | RAM | + | MCP47FVB22 | 12-bit | 2 | 1 | RAM | + |------------|--------------|-------------|-------------|------------| + | MCP47FVB04 | 8-bit | 4 | 2 | RAM | + | MCP47FVB14 | 10-bit | 4 | 2 | RAM | + | MCP47FVB24 | 12-bit | 4 | 2 | RAM | + |------------|--------------|-------------|-------------|------------| + | MCP47FVB08 | 8-bit | 8 | 2 | RAM | + | MCP47FVB18 | 10-bit | 8 | 2 | RAM | + | MCP47FVB28 | 12-bit | 8 | 2 | RAM | + |------------|--------------|-------------|-------------|------------| + | MCP47FEB04 | 8-bit | 4 | 2 | EEPROM | + | MCP47FEB14 | 10-bit | 4 | 2 | EEPROM | + | MCP47FEB24 | 12-bit | 4 | 2 | EEPROM | + |------------|--------------|-------------|-------------|------------| + | MCP47FEB08 | 8-bit | 8 | 2 | EEPROM | + | MCP47FEB18 | 10-bit | 8 | 2 | EEPROM | + | MCP47FEB28 | 12-bit | 8 | 2 | EEPROM | + +------------+--------------+-------------+-------------+------------+ + +properties: + compatible: + enum: + - microchip,mcp47feb01 + - microchip,mcp47feb11 + - microchip,mcp47feb21 + - microchip,mcp47feb02 + - microchip,mcp47feb12 + - microchip,mcp47feb22 + - microchip,mcp47fvb01 + - microchip,mcp47fvb11 + - microchip,mcp47fvb21 + - microchip,mcp47fvb02 + - microchip,mcp47fvb12 + - microchip,mcp47fvb22 + - microchip,mcp47fvb04 + - microchip,mcp47fvb14 + - microchip,mcp47fvb24 + - microchip,mcp47fvb08 + - microchip,mcp47fvb18 + - microchip,mcp47fvb28 + - microchip,mcp47feb04 + - microchip,mcp47feb14 + - microchip,mcp47feb24 + - microchip,mcp47feb08 + - microchip,mcp47feb18 + - microchip,mcp47feb28 + + reg: + maxItems: 1 + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + + vdd-supply: + description: | + Provides power and it will be used as the reference voltage if vref-supply + is not provided. + + vref-supply: + description: | + Vref pin is used as a voltage reference when this supply is specified. + Into the datasheet it could be found as a Vref0. + If it does not exists the internal reference will be used. + This will be used as a reference voltage for the following outputs: + - for single-channel device: Vout0; + - for dual-channel device: Vout0, Vout1; + - for quad-channel device: Vout0, Vout2; + - for octal-channel device: Vout0, Vout2, Vout6, Vout8; + + vref1-supply: + description: | + Vref1 pin is used as a voltage reference when this supply is specified. + If it does not exists the internal reference will be used. + This will be used as a reference voltage for the following outputs: + - for quad-channel device: Vout1, Vout3; + - for octal-channel device: Vout1, Vout3, Vout5, Vout7; + + lat-gpios: + description: | + LAT pin to be used as a hardware trigger to synchronously update the DAC + channels and the pin is active Low. It could be also found as lat0 in + datasheet. + maxItems: 1 + + lat1-gpios: + description: | + LAT1 pin to be used as a hardware trigger to synchronously update the odd + DAC channels on devices with 4 and 8 channels. The pin is active Low. + maxItems: 1 + + microchip,vref-buffered: + type: boolean + description: | + Enable buffering of the external Vref/Vref0 pin in cases where the + external reference voltage does not have sufficient current capability in + order not to drop it’s voltage when connected to the internal resistor + ladder circuit. + + microchip,vref1-buffered: + type: boolean + description: | + Enable buffering of the external Vref1 pin in cases where the external + reference voltage does not have sufficient current capability in order not + to drop it’s voltage when connected to the internal resistor ladder + circuit. + + microchip,output-gain-2x: + type: boolean + description: | + +patternProperties: + "^channel@[0-7]$": + $ref: dac.yaml + type: object + description: Voltage output channel. + + properties: + reg: + description: The channel number. + minimum: 1 + maximum: 7 + + label: + description: Unique name to identify which channel this is. + + required: + - reg + + unevaluatedProperties: false + +required: + - compatible + - reg + - vdd-supply + +allOf: + - if: + properties: + compatible: + contains: + enum: + - microchip,mcp47feb01 + - microchip,mcp47feb11 + - microchip,mcp47feb21 + - microchip,mcp47fvb01 + - microchip,mcp47fvb11 + - microchip,mcp47fvb21 + then: + properties: + lat-gpios: true + lat1-gpios: false + vref-supply: true + vref1-supply: false + microchip,vref-buffered: true + microchip,vref1-buffered: false + patternProperties: + "^channel@[1]$": + properties: + reg: + items: + maximum: 1 + "^channel@[2-7]$": false + - if: + properties: + compatible: + contains: + enum: + - microchip,mcp47feb02 + - microchip,mcp47feb12 + - microchip,mcp47feb22 + - microchip,mcp47fvb02 + - microchip,mcp47fvb12 + - microchip,mcp47fvb22 + then: + properties: + lat-gpios: true + lat1-gpios: false + vref-supply: true + vref1-supply: false + microchip,vref-buffered: true + microchip,vref1-buffered: false + patternProperties: + "^channel@[1-2]$": + properties: + reg: + items: + maximum: 1 + "^channel@[3-7]$": false + - if: + properties: + compatible: + contains: + enum: + - microchip,mcp47fvb04 + - microchip,mcp47fvb14 + - microchip,mcp47fvb24 + - microchip,mcp47fvb08 + - microchip,mcp47fvb18 + - microchip,mcp47fvb28 + - microchip,mcp47feb04 + - microchip,mcp47feb14 + - microchip,mcp47feb24 + - microchip,mcp47feb08 + - microchip,mcp47feb18 + - microchip,mcp47feb28 + then: + properties: + lat-gpios: true + lat1-gpios: true + vref-supply: true + vref1-supply: true + microchip,vref-buffered: true + microchip,vref1-buffered: true + patternProperties: + "^channel@[1-4]$": + properties: + reg: + items: + maximum: 1 + "^channel@[5-7]$": false + - if: + not: + required: + - vref-supply + then: + properties: + microchip,vref-buffered: false + - if: + not: + required: + - vref1-supply + then: + properties: + microchip,vref1-buffered: false + +additionalProperties: false + +examples: + - | + i2c { + + #address-cells = <1>; + #size-cells = <0>; + dac@0 { + compatible = "microchip,mcp47feb02"; + reg = <0>; + vdd-supply = <&vdac_vdd>; + vref-supply = <&vref_reg>; + + #address-cells = <1>; + #size-cells = <0>; + channel@0 { + reg = <0>; + label = "DAC_OUTPUT_0"; + }; + + channel@1 { + reg = <0x1>; + label = "DAC_OUTPUT_1"; + }; + }; + }; +... diff --git a/MAINTAINERS b/MAINTAINERS index a92290fffa163f9fe8fe3f04bf66426f9a894409..6f51890cfc3081bc49c08fddc8af526c1ecc8d72 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14938,6 +14938,12 @@ F: Documentation/ABI/testing/sysfs-bus-iio-potentiometer-mcp4531 F: drivers/iio/potentiometer/mcp4018.c F: drivers/iio/potentiometer/mcp4531.c +MCP47FEB02 MICROCHIP DAC DRIVER +M: Ariana Lazar <ariana.lazar@microchip.com> +L: linux-iio@vger.kernel.org +S: Supported +F: Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml + MCP4821 DAC DRIVER M: Anshul Dalal <anshulusr@gmail.com> L: linux-iio@vger.kernel.org -- 2.43.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: dac: adding support for Microchip MCP47FEB02 2025-09-22 11:30 ` [PATCH 1/2] dt-bindings: iio: dac: adding " Ariana Lazar @ 2025-09-22 16:38 ` Rob Herring (Arm) 2025-09-22 21:59 ` David Lechner 2025-09-22 22:00 ` Rob Herring 2 siblings, 0 replies; 14+ messages in thread From: Rob Herring (Arm) @ 2025-09-22 16:38 UTC (permalink / raw) To: Ariana Lazar Cc: linux-iio, Nuno Sá, Andy Shevchenko, Krzysztof Kozlowski, David Lechner, devicetree, Jonathan Cameron, Conor Dooley, linux-kernel On Mon, 22 Sep 2025 14:30:53 +0300, Ariana Lazar wrote: > This is the device tree schema for iio driver for Microchip > MCP47F(E/V)B(0/1/2)1, MCP47F(E/V)B(0/1/2)2, MCP47F(E/V)B(0/1/2)4 and > MCP47F(E/V)B(0/1/2)8 series of buffered voltage output Digital-to-Analog > Converters with nonvolatile or volatile memory and an I2C Interface. > > The families support up to 8 output channels. > > The devices can be 8-bit, 10-bit and 12-bit. > > Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com> > --- > .../bindings/iio/dac/microchip,mcp47feb02.yaml | 305 +++++++++++++++++++++ > MAINTAINERS | 6 + > 2 files changed, 311 insertions(+) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: ./Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml:199:8: [warning] wrong indentation: expected 8 but found 7 (indentation) ./Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml:200:9: [warning] wrong indentation: expected 9 but found 8 (indentation) ./Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml:201:10: [warning] wrong indentation: expected 10 but found 9 (indentation) ./Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml:202:11: [warning] wrong indentation: expected 11 but found 10 (indentation) ./Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml:225:8: [warning] wrong indentation: expected 8 but found 7 (indentation) ./Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml:226:9: [warning] wrong indentation: expected 9 but found 8 (indentation) ./Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml:227:10: [warning] wrong indentation: expected 10 but found 9 (indentation) ./Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml:228:11: [warning] wrong indentation: expected 11 but found 10 (indentation) ./Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml:257:8: [warning] wrong indentation: expected 8 but found 7 (indentation) ./Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml:258:9: [warning] wrong indentation: expected 9 but found 8 (indentation) ./Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml:259:10: [warning] wrong indentation: expected 10 but found 9 (indentation) ./Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml:260:11: [warning] wrong indentation: expected 11 but found 10 (indentation) dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml: allOf:0:then:patternProperties:^channel@[1]$: 'anyOf' conditional failed, one must be fixed: '^channel@[2-7]$' is not one of ['$ref', 'additionalItems', 'additionalProperties', 'allOf', 'anyOf', 'const', 'contains', 'default', 'dependencies', 'dependentRequired', 'dependentSchemas', 'deprecated', 'description', 'else', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'items', 'if', 'minItems', 'minimum', 'maxItems', 'maximum', 'multipleOf', 'not', 'oneOf', 'pattern', 'patternProperties', 'properties', 'required', 'then', 'typeSize', 'unevaluatedProperties', 'uniqueItems'] 'type' was expected from schema $id: http://devicetree.org/meta-schemas/keywords.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml: allOf:1:then:patternProperties:^channel@[1-2]$: 'anyOf' conditional failed, one must be fixed: '^channel@[3-7]$' is not one of ['$ref', 'additionalItems', 'additionalProperties', 'allOf', 'anyOf', 'const', 'contains', 'default', 'dependencies', 'dependentRequired', 'dependentSchemas', 'deprecated', 'description', 'else', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'items', 'if', 'minItems', 'minimum', 'maxItems', 'maximum', 'multipleOf', 'not', 'oneOf', 'pattern', 'patternProperties', 'properties', 'required', 'then', 'typeSize', 'unevaluatedProperties', 'uniqueItems'] 'type' was expected from schema $id: http://devicetree.org/meta-schemas/keywords.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.example.dtb: dac@0 (microchip,mcp47feb02): channel@0:reg:0:0: 0 is less than the minimum of 1 from schema $id: http://devicetree.org/schemas/iio/dac/microchip,mcp47feb02.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250922-mcp47feb02-v1-1-06cb4acaa347@microchip.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: dac: adding support for Microchip MCP47FEB02 2025-09-22 11:30 ` [PATCH 1/2] dt-bindings: iio: dac: adding " Ariana Lazar 2025-09-22 16:38 ` Rob Herring (Arm) @ 2025-09-22 21:59 ` David Lechner 2025-09-27 17:00 ` Jonathan Cameron 2025-09-22 22:00 ` Rob Herring 2 siblings, 1 reply; 14+ messages in thread From: David Lechner @ 2025-09-22 21:59 UTC (permalink / raw) To: Ariana Lazar, Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-iio, devicetree, linux-kernel On 9/22/25 6:30 AM, Ariana Lazar wrote: > This is the device tree schema for iio driver for Microchip > MCP47F(E/V)B(0/1/2)1, MCP47F(E/V)B(0/1/2)2, MCP47F(E/V)B(0/1/2)4 and > MCP47F(E/V)B(0/1/2)8 series of buffered voltage output Digital-to-Analog > Converters with nonvolatile or volatile memory and an I2C Interface. > > The families support up to 8 output channels. > > The devices can be 8-bit, 10-bit and 12-bit. > > Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com> > --- > .../bindings/iio/dac/microchip,mcp47feb02.yaml | 305 +++++++++++++++++++++ > MAINTAINERS | 6 + > 2 files changed, 311 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml b/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml > new file mode 100644 > index 0000000000000000000000000000000000000000..d05ddafa37540bc1f6b6ce65a466b95913925c10 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml > @@ -0,0 +1,305 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/dac/microchip,mcp47feb02.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Microchip MCP47F(E/V)B(0/1/2)(1/2/4/8) DAC with I2C Interface Families > + > +maintainers: > + - Ariana Lazar <ariana.lazar@microchip.com> > + > +description: | > + Datasheet for MCP47FEB01, MCP47FEB11, MCP47FEB21, MCP47FEB02, MCP47FEB12, > + MCP47FEB22 can be found here: > + https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005375A.pdf > + Datasheet for MCP47FVBXX can be found here: > + https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005405A.pdf > + Datasheet for MCP47FEB04, MCP47FEB14, MCP47FEB24, MCP47FEB08, MCP47FEB18, > + MCP47FEB28, MCP47FVB04, MCP47FVB14, MCP47FVB24, MCP47FVB08, MCP47FVB18, > + MCP47FVB28 can be found here: > + https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/MCP47FXBX48-Data-Sheet-DS200006368A.pdf > + > + +------------+--------------+-------------+-------------+------------+ > + | Device | Resolution | Channels | Vref number | Memory | > + |------------|--------------|-------------|-------------|------------| > + | MCP47FEB01 | 8-bit | 1 | 1 | EEPROM | > + | MCP47FEB11 | 10-bit | 1 | 1 | EEPROM | > + | MCP47FEB21 | 12-bit | 1 | 1 | EEPROM | > + |------------|--------------|-------------|-------------|------------| > + | MCP47FEB02 | 8-bit | 2 | 1 | EEPROM | > + | MCP47FEB12 | 10-bit | 2 | 1 | EEPROM | > + | MCP47FEB22 | 12-bit | 2 | 1 | EEPROM | > + |------------|--------------|-------------|-------------|------------| > + | MCP47FVB01 | 8-bit | 1 | 1 | RAM | > + | MCP47FVB11 | 10-bit | 1 | 1 | RAM | > + | MCP47FVB21 | 12-bit | 1 | 1 | RAM | > + |------------|--------------|-------------|-------------|------------| > + | MCP47FVB02 | 8-bit | 2 | 1 | RAM | > + | MCP47FVB12 | 10-bit | 2 | 1 | RAM | > + | MCP47FVB22 | 12-bit | 2 | 1 | RAM | > + |------------|--------------|-------------|-------------|------------| > + | MCP47FVB04 | 8-bit | 4 | 2 | RAM | > + | MCP47FVB14 | 10-bit | 4 | 2 | RAM | > + | MCP47FVB24 | 12-bit | 4 | 2 | RAM | > + |------------|--------------|-------------|-------------|------------| > + | MCP47FVB08 | 8-bit | 8 | 2 | RAM | > + | MCP47FVB18 | 10-bit | 8 | 2 | RAM | > + | MCP47FVB28 | 12-bit | 8 | 2 | RAM | > + |------------|--------------|-------------|-------------|------------| > + | MCP47FEB04 | 8-bit | 4 | 2 | EEPROM | > + | MCP47FEB14 | 10-bit | 4 | 2 | EEPROM | > + | MCP47FEB24 | 12-bit | 4 | 2 | EEPROM | > + |------------|--------------|-------------|-------------|------------| > + | MCP47FEB08 | 8-bit | 8 | 2 | EEPROM | > + | MCP47FEB18 | 10-bit | 8 | 2 | EEPROM | > + | MCP47FEB28 | 12-bit | 8 | 2 | EEPROM | > + +------------+--------------+-------------+-------------+------------+ > + > +properties: > + compatible: > + enum: > + - microchip,mcp47feb01 > + - microchip,mcp47feb11 > + - microchip,mcp47feb21 > + - microchip,mcp47feb02 > + - microchip,mcp47feb12 > + - microchip,mcp47feb22 > + - microchip,mcp47fvb01 > + - microchip,mcp47fvb11 > + - microchip,mcp47fvb21 > + - microchip,mcp47fvb02 > + - microchip,mcp47fvb12 > + - microchip,mcp47fvb22 > + - microchip,mcp47fvb04 > + - microchip,mcp47fvb14 > + - microchip,mcp47fvb24 > + - microchip,mcp47fvb08 > + - microchip,mcp47fvb18 > + - microchip,mcp47fvb28 > + - microchip,mcp47feb04 > + - microchip,mcp47feb14 > + - microchip,mcp47feb24 > + - microchip,mcp47feb08 > + - microchip,mcp47feb18 > + - microchip,mcp47feb28 > + > + reg: > + maxItems: 1 > + > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > + vdd-supply: > + description: | Don't need `|` unless formatting needs to be preserved, like for the table in the description above or the bullet lists below. > + Provides power and it will be used as the reference voltage if vref-supply > + is not provided. > + > + vref-supply: > + description: | > + Vref pin is used as a voltage reference when this supply is specified. > + Into the datasheet it could be found as a Vref0. > + If it does not exists the internal reference will be used. It looks like there is also the possibility to use V_DD as the reference voltage. Not sure the best way to handle that though. > + This will be used as a reference voltage for the following outputs: > + - for single-channel device: Vout0; > + - for dual-channel device: Vout0, Vout1; > + - for quad-channel device: Vout0, Vout2; > + - for octal-channel device: Vout0, Vout2, Vout6, Vout8; > + > + vref1-supply: > + description: | > + Vref1 pin is used as a voltage reference when this supply is specified. > + If it does not exists the internal reference will be used. > + This will be used as a reference voltage for the following outputs: > + - for quad-channel device: Vout1, Vout3; > + - for octal-channel device: Vout1, Vout3, Vout5, Vout7; > + > + lat-gpios: > + description: | > + LAT pin to be used as a hardware trigger to synchronously update the DAC > + channels and the pin is active Low. It could be also found as lat0 in > + datasheet. > + maxItems: 1 > + > + lat1-gpios: > + description: | > + LAT1 pin to be used as a hardware trigger to synchronously update the odd > + DAC channels on devices with 4 and 8 channels. The pin is active Low. > + maxItems: 1 > + > + microchip,vref-buffered: > + type: boolean > + description: | > + Enable buffering of the external Vref/Vref0 pin in cases where the > + external reference voltage does not have sufficient current capability in > + order not to drop it’s voltage when connected to the internal resistor > + ladder circuit. > + > + microchip,vref1-buffered: > + type: boolean > + description: | > + Enable buffering of the external Vref1 pin in cases where the external > + reference voltage does not have sufficient current capability in order not > + to drop it’s voltage when connected to the internal resistor ladder > + circuit. > + > + microchip,output-gain-2x: > + type: boolean > + description: | > + > +patternProperties: > + "^channel@[0-7]$": > + $ref: dac.yaml > + type: object > + description: Voltage output channel. > + > + properties: > + reg: > + description: The channel number. > + minimum: 1 > + maximum: 7 > + > + label: > + description: Unique name to identify which channel this is. > + > + required: > + - reg > + > + unevaluatedProperties: false > + > +required: > + - compatible > + - reg > + - vdd-supply > + > +allOf: > + - if: > + properties: > + compatible: > + contains: > + enum: > + - microchip,mcp47feb01 > + - microchip,mcp47feb11 > + - microchip,mcp47feb21 > + - microchip,mcp47fvb01 > + - microchip,mcp47fvb11 > + - microchip,mcp47fvb21 > + then: > + properties: > + lat-gpios: true > + lat1-gpios: false > + vref-supply: true > + vref1-supply: false > + microchip,vref-buffered: true Everything is already true, so we can leave out all of those lines. It make it hard to see what is actually being modified. > + microchip,vref1-buffered: false > + patternProperties: > + "^channel@[1]$": > + properties: > + reg: > + items: No items: here. > + maximum: 1 > + "^channel@[2-7]$": false > + - if: > + properties: > + compatible: > + contains: > + enum: > + - microchip,mcp47feb02 > + - microchip,mcp47feb12 > + - microchip,mcp47feb22 > + - microchip,mcp47fvb02 > + - microchip,mcp47fvb12 > + - microchip,mcp47fvb22 > + then: > + properties: > + lat-gpios: true > + lat1-gpios: false > + vref-supply: true > + vref1-supply: false > + microchip,vref-buffered: true > + microchip,vref1-buffered: false > + patternProperties: > + "^channel@[1-2]$": > + properties: > + reg: > + items: > + maximum: 1 Isn't maximum 2 in this case? > + "^channel@[3-7]$": false > + - if: > + properties: > + compatible: > + contains: > + enum: > + - microchip,mcp47fvb04 > + - microchip,mcp47fvb14 > + - microchip,mcp47fvb24 > + - microchip,mcp47fvb08 > + - microchip,mcp47fvb18 > + - microchip,mcp47fvb28 > + - microchip,mcp47feb04 > + - microchip,mcp47feb14 > + - microchip,mcp47feb24 > + - microchip,mcp47feb08 > + - microchip,mcp47feb18 > + - microchip,mcp47feb28 Should the 8-channel chips be include here? This limits the channel@ to 1-4, so that doesn't jive with having 8 channels. > + then: > + properties: > + lat-gpios: true > + lat1-gpios: true > + vref-supply: true > + vref1-supply: true > + microchip,vref-buffered: true > + microchip,vref1-buffered: true > + patternProperties: > + "^channel@[1-4]$": > + properties: > + reg: > + items: > + maximum: 1 And 4 here? > + "^channel@[5-7]$": false > + - if: > + not: > + required: > + - vref-supply > + then: > + properties: > + microchip,vref-buffered: false > + - if: > + not: > + required: > + - vref1-supply > + then: > + properties: > + microchip,vref1-buffered: false > + > +additionalProperties: false > + > +examples: > + - | > + i2c { > + > + #address-cells = <1>; > + #size-cells = <0>; > + dac@0 { > + compatible = "microchip,mcp47feb02"; > + reg = <0>; > + vdd-supply = <&vdac_vdd>; > + vref-supply = <&vref_reg>; > + > + #address-cells = <1>; > + #size-cells = <0>; > + channel@0 { > + reg = <0>; > + label = "DAC_OUTPUT_0"; These aren't particularly useful labels. It gives the same info as the register number. > + }; > + > + channel@1 { > + reg = <0x1>; > + label = "DAC_OUTPUT_1"; > + }; > + }; > + }; > +... ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: dac: adding support for Microchip MCP47FEB02 2025-09-22 21:59 ` David Lechner @ 2025-09-27 17:00 ` Jonathan Cameron 0 siblings, 0 replies; 14+ messages in thread From: Jonathan Cameron @ 2025-09-27 17:00 UTC (permalink / raw) To: David Lechner Cc: Ariana Lazar, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio, devicetree, linux-kernel > > + vref-supply: > > + description: | > > + Vref pin is used as a voltage reference when this supply is specified. > > + Into the datasheet it could be found as a Vref0. > > + If it does not exists the internal reference will be used. > > It looks like there is also the possibility to use V_DD as the reference > voltage. Not sure the best way to handle that though. Indeed that's awkward. I suppose a custom property as choice of VDD or internal reference is unusual and it might be critical to how what is downstream of the DAC so we can't even do it via userspace _scale control. I'm lazy so haven't checked the values. If the internal reference is near VDD it is probably safe enough to just make this a userspace problem if anyone ever needs it. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: dac: adding support for Microchip MCP47FEB02 2025-09-22 11:30 ` [PATCH 1/2] dt-bindings: iio: dac: adding " Ariana Lazar 2025-09-22 16:38 ` Rob Herring (Arm) 2025-09-22 21:59 ` David Lechner @ 2025-09-22 22:00 ` Rob Herring 2 siblings, 0 replies; 14+ messages in thread From: Rob Herring @ 2025-09-22 22:00 UTC (permalink / raw) To: Ariana Lazar Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Krzysztof Kozlowski, Conor Dooley, linux-iio, devicetree, linux-kernel On Mon, Sep 22, 2025 at 02:30:53PM +0300, Ariana Lazar wrote: > This is the device tree schema for iio driver for Microchip > MCP47F(E/V)B(0/1/2)1, MCP47F(E/V)B(0/1/2)2, MCP47F(E/V)B(0/1/2)4 and > MCP47F(E/V)B(0/1/2)8 series of buffered voltage output Digital-to-Analog > Converters with nonvolatile or volatile memory and an I2C Interface. > > The families support up to 8 output channels. > > The devices can be 8-bit, 10-bit and 12-bit. > > Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com> > --- > .../bindings/iio/dac/microchip,mcp47feb02.yaml | 305 +++++++++++++++++++++ > MAINTAINERS | 6 + > 2 files changed, 311 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml b/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml > new file mode 100644 > index 0000000000000000000000000000000000000000..d05ddafa37540bc1f6b6ce65a466b95913925c10 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml > @@ -0,0 +1,305 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/dac/microchip,mcp47feb02.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Microchip MCP47F(E/V)B(0/1/2)(1/2/4/8) DAC with I2C Interface Families > + > +maintainers: > + - Ariana Lazar <ariana.lazar@microchip.com> > + > +description: | > + Datasheet for MCP47FEB01, MCP47FEB11, MCP47FEB21, MCP47FEB02, MCP47FEB12, > + MCP47FEB22 can be found here: > + https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005375A.pdf > + Datasheet for MCP47FVBXX can be found here: > + https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005405A.pdf > + Datasheet for MCP47FEB04, MCP47FEB14, MCP47FEB24, MCP47FEB08, MCP47FEB18, > + MCP47FEB28, MCP47FVB04, MCP47FVB14, MCP47FVB24, MCP47FVB08, MCP47FVB18, > + MCP47FVB28 can be found here: > + https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/MCP47FXBX48-Data-Sheet-DS200006368A.pdf > + > + +------------+--------------+-------------+-------------+------------+ > + | Device | Resolution | Channels | Vref number | Memory | > + |------------|--------------|-------------|-------------|------------| > + | MCP47FEB01 | 8-bit | 1 | 1 | EEPROM | > + | MCP47FEB11 | 10-bit | 1 | 1 | EEPROM | > + | MCP47FEB21 | 12-bit | 1 | 1 | EEPROM | > + |------------|--------------|-------------|-------------|------------| > + | MCP47FEB02 | 8-bit | 2 | 1 | EEPROM | > + | MCP47FEB12 | 10-bit | 2 | 1 | EEPROM | > + | MCP47FEB22 | 12-bit | 2 | 1 | EEPROM | > + |------------|--------------|-------------|-------------|------------| > + | MCP47FVB01 | 8-bit | 1 | 1 | RAM | > + | MCP47FVB11 | 10-bit | 1 | 1 | RAM | > + | MCP47FVB21 | 12-bit | 1 | 1 | RAM | > + |------------|--------------|-------------|-------------|------------| > + | MCP47FVB02 | 8-bit | 2 | 1 | RAM | > + | MCP47FVB12 | 10-bit | 2 | 1 | RAM | > + | MCP47FVB22 | 12-bit | 2 | 1 | RAM | > + |------------|--------------|-------------|-------------|------------| > + | MCP47FVB04 | 8-bit | 4 | 2 | RAM | > + | MCP47FVB14 | 10-bit | 4 | 2 | RAM | > + | MCP47FVB24 | 12-bit | 4 | 2 | RAM | > + |------------|--------------|-------------|-------------|------------| > + | MCP47FVB08 | 8-bit | 8 | 2 | RAM | > + | MCP47FVB18 | 10-bit | 8 | 2 | RAM | > + | MCP47FVB28 | 12-bit | 8 | 2 | RAM | > + |------------|--------------|-------------|-------------|------------| > + | MCP47FEB04 | 8-bit | 4 | 2 | EEPROM | > + | MCP47FEB14 | 10-bit | 4 | 2 | EEPROM | > + | MCP47FEB24 | 12-bit | 4 | 2 | EEPROM | > + |------------|--------------|-------------|-------------|------------| > + | MCP47FEB08 | 8-bit | 8 | 2 | EEPROM | > + | MCP47FEB18 | 10-bit | 8 | 2 | EEPROM | > + | MCP47FEB28 | 12-bit | 8 | 2 | EEPROM | > + +------------+--------------+-------------+-------------+------------+ > + > +properties: > + compatible: > + enum: > + - microchip,mcp47feb01 > + - microchip,mcp47feb11 > + - microchip,mcp47feb21 > + - microchip,mcp47feb02 > + - microchip,mcp47feb12 > + - microchip,mcp47feb22 > + - microchip,mcp47fvb01 > + - microchip,mcp47fvb11 > + - microchip,mcp47fvb21 > + - microchip,mcp47fvb02 > + - microchip,mcp47fvb12 > + - microchip,mcp47fvb22 > + - microchip,mcp47fvb04 > + - microchip,mcp47fvb14 > + - microchip,mcp47fvb24 > + - microchip,mcp47fvb08 > + - microchip,mcp47fvb18 > + - microchip,mcp47fvb28 > + - microchip,mcp47feb04 > + - microchip,mcp47feb14 > + - microchip,mcp47feb24 > + - microchip,mcp47feb08 > + - microchip,mcp47feb18 > + - microchip,mcp47feb28 > + > + reg: > + maxItems: 1 > + > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > + vdd-supply: > + description: | Don't need '|' if no formatting to preserve. > + Provides power and it will be used as the reference voltage if vref-supply > + is not provided. > + > + vref-supply: > + description: | > + Vref pin is used as a voltage reference when this supply is specified. > + Into the datasheet it could be found as a Vref0. > + If it does not exists the internal reference will be used. blank line between paragraphs. But is every sentence a paragraph? > + This will be used as a reference voltage for the following outputs: > + - for single-channel device: Vout0; > + - for dual-channel device: Vout0, Vout1; > + - for quad-channel device: Vout0, Vout2; > + - for octal-channel device: Vout0, Vout2, Vout6, Vout8; > + > + vref1-supply: > + description: | > + Vref1 pin is used as a voltage reference when this supply is specified. > + If it does not exists the internal reference will be used. > + This will be used as a reference voltage for the following outputs: > + - for quad-channel device: Vout1, Vout3; > + - for octal-channel device: Vout1, Vout3, Vout5, Vout7; > + > + lat-gpios: > + description: | > + LAT pin to be used as a hardware trigger to synchronously update the DAC > + channels and the pin is active Low. It could be also found as lat0 in > + datasheet. > + maxItems: 1 > + > + lat1-gpios: > + description: | > + LAT1 pin to be used as a hardware trigger to synchronously update the odd > + DAC channels on devices with 4 and 8 channels. The pin is active Low. > + maxItems: 1 > + > + microchip,vref-buffered: > + type: boolean > + description: | > + Enable buffering of the external Vref/Vref0 pin in cases where the > + external reference voltage does not have sufficient current capability in > + order not to drop it’s voltage when connected to the internal resistor > + ladder circuit. > + > + microchip,vref1-buffered: > + type: boolean > + description: | > + Enable buffering of the external Vref1 pin in cases where the external > + reference voltage does not have sufficient current capability in order not > + to drop it’s voltage when connected to the internal resistor ladder > + circuit. > + > + microchip,output-gain-2x: > + type: boolean > + description: | ? > + > +patternProperties: > + "^channel@[0-7]$": > + $ref: dac.yaml > + type: object > + description: Voltage output channel. > + > + properties: > + reg: > + description: The channel number. > + minimum: 1 > + maximum: 7 > + > + label: > + description: Unique name to identify which channel this is. > + > + required: > + - reg > + > + unevaluatedProperties: false > + > +required: > + - compatible > + - reg > + - vdd-supply > + > +allOf: > + - if: > + properties: > + compatible: > + contains: > + enum: > + - microchip,mcp47feb01 > + - microchip,mcp47feb11 > + - microchip,mcp47feb21 > + - microchip,mcp47fvb01 > + - microchip,mcp47fvb11 > + - microchip,mcp47fvb21 > + then: > + properties: > + lat-gpios: true true has no effect as the property is already allowed. Drop all the true cases. > + lat1-gpios: false > + vref-supply: true > + vref1-supply: false > + microchip,vref-buffered: true > + microchip,vref1-buffered: false > + patternProperties: > + "^channel@[1]$": Did you mean "[01]"? If not, that's not a pattern, but a fixed string. > + properties: > + reg: > + items: > + maximum: 1 > + "^channel@[2-7]$": false > + - if: > + properties: > + compatible: > + contains: > + enum: > + - microchip,mcp47feb02 > + - microchip,mcp47feb12 > + - microchip,mcp47feb22 > + - microchip,mcp47fvb02 > + - microchip,mcp47fvb12 > + - microchip,mcp47fvb22 > + then: > + properties: > + lat-gpios: true > + lat1-gpios: false > + vref-supply: true > + vref1-supply: false > + microchip,vref-buffered: true > + microchip,vref1-buffered: false > + patternProperties: > + "^channel@[1-2]$": > + properties: > + reg: > + items: > + maximum: 1 Based on the unit-address, the minimum is 1 and the maximum is 2. "enum: [ 1, 2 ]" is a bit more concise. > + "^channel@[3-7]$": false > + - if: > + properties: > + compatible: > + contains: > + enum: > + - microchip,mcp47fvb04 > + - microchip,mcp47fvb14 > + - microchip,mcp47fvb24 > + - microchip,mcp47fvb08 > + - microchip,mcp47fvb18 > + - microchip,mcp47fvb28 > + - microchip,mcp47feb04 > + - microchip,mcp47feb14 > + - microchip,mcp47feb24 > + - microchip,mcp47feb08 > + - microchip,mcp47feb18 > + - microchip,mcp47feb28 > + then: > + properties: > + lat-gpios: true > + lat1-gpios: true > + vref-supply: true > + vref1-supply: true > + microchip,vref-buffered: true > + microchip,vref1-buffered: true > + patternProperties: > + "^channel@[1-4]$": > + properties: > + reg: > + items: > + maximum: 1 ? > + "^channel@[5-7]$": false > + - if: > + not: > + required: > + - vref-supply > + then: > + properties: > + microchip,vref-buffered: false > + - if: > + not: > + required: > + - vref1-supply > + then: > + properties: > + microchip,vref1-buffered: false > + > +additionalProperties: false > + > +examples: > + - | > + i2c { > + > + #address-cells = <1>; > + #size-cells = <0>; > + dac@0 { > + compatible = "microchip,mcp47feb02"; > + reg = <0>; > + vdd-supply = <&vdac_vdd>; > + vref-supply = <&vref_reg>; > + > + #address-cells = <1>; > + #size-cells = <0>; > + channel@0 { > + reg = <0>; > + label = "DAC_OUTPUT_0"; > + }; > + > + channel@1 { > + reg = <0x1>; > + label = "DAC_OUTPUT_1"; > + }; > + }; > + }; > +... > diff --git a/MAINTAINERS b/MAINTAINERS > index a92290fffa163f9fe8fe3f04bf66426f9a894409..6f51890cfc3081bc49c08fddc8af526c1ecc8d72 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -14938,6 +14938,12 @@ F: Documentation/ABI/testing/sysfs-bus-iio-potentiometer-mcp4531 > F: drivers/iio/potentiometer/mcp4018.c > F: drivers/iio/potentiometer/mcp4531.c > > +MCP47FEB02 MICROCHIP DAC DRIVER > +M: Ariana Lazar <ariana.lazar@microchip.com> > +L: linux-iio@vger.kernel.org > +S: Supported > +F: Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml > + > MCP4821 DAC DRIVER > M: Anshul Dalal <anshulusr@gmail.com> > L: linux-iio@vger.kernel.org > > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/2] iio: dac: adding support for Microchip MCP47FEB02 2025-09-22 11:30 [PATCH 0/2] Adding support for Microchip MCP47FEB02 Ariana Lazar 2025-09-22 11:30 ` [PATCH 1/2] dt-bindings: iio: dac: adding " Ariana Lazar @ 2025-09-22 11:30 ` Ariana Lazar 2025-09-22 20:10 ` Nuno Sá ` (2 more replies) 1 sibling, 3 replies; 14+ messages in thread From: Ariana Lazar @ 2025-09-22 11:30 UTC (permalink / raw) To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-iio, devicetree, linux-kernel, Ariana Lazar This is the iio driver for Microchip MCP47F(E/V)B(0/1/2)1, MCP47F(E/V)B(0/1/2)2, MCP47F(E/V)B(0/1/2)4 and MCP47F(E/V)B(0/1/2)8 series of buffered voltage output Digital-to-Analog Converters with nonvolatile or volatile memory and an I2C Interface. The families support up to 8 output channels. The devices can be 8-bit, 10-bit and 12-bit. Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com> --- MAINTAINERS | 1 + drivers/iio/dac/Kconfig | 16 + drivers/iio/dac/Makefile | 1 + drivers/iio/dac/mcp47feb02.c | 1347 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 1365 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 6f51890cfc3081bc49c08fddc8af526c1ecc8d72..0f97f90ac2f492895d27da86d831df83cb402516 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14943,6 +14943,7 @@ M: Ariana Lazar <ariana.lazar@microchip.com> L: linux-iio@vger.kernel.org S: Supported F: Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml +F: drivers/iio/dac/mcp47feb02.c MCP4821 DAC DRIVER M: Anshul Dalal <anshulusr@gmail.com> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig index e0996dc014a3d538ab6b4e0d50ff54ede50f1527..179ce565036e3494e4ce43bb926de31f38b547c4 100644 --- a/drivers/iio/dac/Kconfig +++ b/drivers/iio/dac/Kconfig @@ -509,6 +509,22 @@ config MCP4728 To compile this driver as a module, choose M here: the module will be called mcp4728. +config MCP47FEB02 + tristate "MCP47F(E/V)B|(0/1/2)(1/2/4/8)DAC driver" + depends on I2C + help + Say yes here if you want to build a driver for the Microchip + MCP47FEB01, MCP47FEB11, MCP47FEB21, MCP47FEB02, MCP47FEB12, + MCP47FEB22, MCP47FVB01, MCP47FVB11, MCP47FVB21, MCP47FVB02, + MCP47FVB12, MCP47FVB02, MCP47FVB12, MCP47FVB22, MCP47FVB04, + MCP47FVB14, MCP47FVB24, MCP47FVB04, MCP47FVB08, MCP47FVB18, + MCP47FVB28, MCP47FEB04, MCP47FEB14 and MCP47FEB24 having up to 8 + channels, 8-bit, 10-bit or 12-bit digital-to-analog converter (DAC) + with I2C interface. + + To compile this driver as a module, choose M here: the module + will be called mcp47feb02. + config MCP4821 tristate "MCP4801/02/11/12/21/22 DAC driver" depends on SPI diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile index 3684cd52b7fa9bc0ad9f855323dcbb2e4965c404..d633a6440fc4b9aba7d8b1c209b6dcd05cd982dd 100644 --- a/drivers/iio/dac/Makefile +++ b/drivers/iio/dac/Makefile @@ -50,6 +50,7 @@ obj-$(CONFIG_MAX5522) += max5522.o obj-$(CONFIG_MAX5821) += max5821.o obj-$(CONFIG_MCP4725) += mcp4725.o obj-$(CONFIG_MCP4728) += mcp4728.o +obj-$(CONFIG_MCP47FEB02) += mcp47feb02.o obj-$(CONFIG_MCP4821) += mcp4821.o obj-$(CONFIG_MCP4922) += mcp4922.o obj-$(CONFIG_STM32_DAC_CORE) += stm32-dac-core.o diff --git a/drivers/iio/dac/mcp47feb02.c b/drivers/iio/dac/mcp47feb02.c new file mode 100644 index 0000000000000000000000000000000000000000..c9c2ded78d9c6013e5618e6342ebc8f50e79a31e --- /dev/null +++ b/drivers/iio/dac/mcp47feb02.c @@ -0,0 +1,1347 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * IIO driver for MCP47FEB02 Multi-Channel DAC with I2C interface + * + * Copyright (C) 2025 Microchip Technology Inc. and its subsidiaries + * + * Author: Ariana Lazar <ariana.lazar@microchip.com> + * + * Datasheet for MCP47FEBXX can be found here: + * https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005375A.pdf + * + * Datasheet for MCP47FVBXX can be found here: + * https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005405A.pdf + * + * Datasheet for MCP47FXBX4/8 can be found here: + * https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/MCP47FXBX48-Data-Sheet-DS200006368A.pdf + */ +#include <linux/bits.h> +#include <linux/bitfield.h> +#include <linux/delay.h> +#include <linux/err.h> +#include <linux/i2c.h> +#include <linux/iio/iio.h> +#include <linux/iio/sysfs.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> + +#define MCP47FEB02_DAC0_REG_ADDR 0x00 +#define MCP47FEB02_DAC1_REG_ADDR 0x01 +#define MCP47FEB02_DAC2_REG_ADDR 0x02 +#define MCP47FEB02_DAC3_REG_ADDR 0x03 +#define MCP47FEB02_DAC4_REG_ADDR 0x04 +#define MCP47FEB02_DAC5_REG_ADDR 0x05 +#define MCP47FEB02_DAC6_REG_ADDR 0x06 +#define MCP47FEB02_DAC7_REG_ADDR 0x07 +#define MCP47FEB02_VREF_REG_ADDR 0x08 +#define MCP47FEB02_POWER_DOWN_REG_ADDR 0x09 +#define MCP47FEB02_GAIN_STATUS_REG_ADDR 0x0A +#define MCP47FEB02_WIPERLOCK_STATUS_REG_ADDR 0x0B + +#define MCP47FEB02_NV_DAC0_REG_ADDR 0x10 +#define MCP47FEB02_NV_DAC1_REG_ADDR 0x11 +#define MCP47FEB02_NV_DAC2_REG_ADDR 0x12 +#define MCP47FEB02_NV_DAC3_REG_ADDR 0x13 +#define MCP47FEB02_NV_DAC4_REG_ADDR 0x14 +#define MCP47FEB02_NV_DAC5_REG_ADDR 0x15 +#define MCP47FEB02_NV_DAC6_REG_ADDR 0x16 +#define MCP47FEB02_NV_DAC7_REG_ADDR 0x17 +#define MCP47FEB02_NV_VREF_REG_ADDR 0x18 +#define MCP47FEB02_NV_POWER_DOWN_REG_ADDR 0x19 +#define MCP47FEB02_NV_GAIN_I2C_SLAVE_REG_ADDR 0x1A + +#define MCP47FEBXX_MAX_CH 8 +#define MCP47FEB02_GAIN_X1 0 +#define MCP47FEB02_GAIN_X2 1 +#define MCP47FEB02_MAX_VALS_SCALES_CH 6 +#define MCP47FEB02_MAX_SCALES_CH 3 +#define MCP47FEB02_DAC_WIPER_UNLOCKED 0 +#define MCP47FEB02_INTERNAL_BAND_GAP_MV 2440 +#define MCP47FEB02_DELAY_1_MS 1000 + +/* Voltage reference, Power-Down control register and DAC Wiperlock status register fields */ +#define CH_0 GENMASK(1, 0) +#define CH_1 GENMASK(3, 2) +#define CH_2 GENMASK(5, 4) +#define CH_3 GENMASK(7, 6) +#define CH_4 GENMASK(9, 8) +#define CH_5 GENMASK(11, 10) +#define CH_6 GENMASK(13, 12) +#define CH_7 GENMASK(15, 14) + +/* Gain Control and I2C Slave Address Reguster fields */ +#define G_0 BIT(8) +#define G_1 BIT(9) +#define G_2 BIT(10) +#define G_3 BIT(11) +#define G_4 BIT(12) +#define G_5 BIT(13) +#define G_6 BIT(14) +#define G_7 BIT(15) + +#define MCP47FEB02_GAIN_STATUS_EEWA_MASK BIT(6) +#define MCP47FEB02_VOLATILE_GAIN_MASK GENMASK(15, 8) +#define MCP47FEB02_NV_I2C_SLAVE_ADDR_MASK GENMASK(7, 0) +#define MCP47FEB02_WIPERLOCK_STATUS_MASK GENMASK(15, 0) +#define CMD_MEM_ADDR_MASK GENMASK(7, 3) + +#define CMD_FORMAT(reg, cmd) (FIELD_PREP(CMD_MEM_ADDR_MASK, (reg)) | (cmd)) +#define READ_CMD 0x06 +#define WRITE_CMD 0x00 +#define READ_OP(ch) (((ch) << 3) | READ_CMD) +#define WRITE_OP(ch) ((ch) << 3) + +enum vref_mode { + MCP47FEB02_VREF_VDD = 0, + MCP47FEB02_INTERNAL_BAND_GAP = 1, + MCP47FEB02_EXTERNAL_VREF_UNBUFFERED = 2, + MCP47FEB02_EXTERNAL_VREF_BUFFERED = 3, +}; + +enum mcp47feb02_scale { + MCP47FEB02_SCALE_VDD = 0, + MCP47FEB02_SCALE_GAIN_X1 = 1, + MCP47FEB02_SCALE_GAIN_X2 = 2, +}; + +enum iio_powerdown_mode { + MCP47FEB02_NORMAL_OPERATION = 0, + MCP47FEB02_IIO_1K = 1, + MCP47FEB02_IIO_100K = 2, + MCP47FEB02_OPEN_CIRCUIT = 3, +}; + +/** + * struct mcp47feb02_features - chip specific data + * @name: device name + * @phys_channels: number of hardware channels + * @resolution: ADC resolution + * @have_ext_vref2: does the hardware have an the second external voltage reference? + * @have_eeprom: does the hardware have an internal eeprom? + */ +struct mcp47feb02_features { + const char *name; + unsigned int phys_channels; + unsigned int resolution; + bool have_ext_vref2; + bool have_eeprom; +}; + +static const struct mcp47feb02_features mcp47feb01_chip_info = { + .name = "mcp47feb01", + .phys_channels = 1, + .resolution = 8, + .have_ext_vref2 = false, + .have_eeprom = true, +}; + +static const struct mcp47feb02_features mcp47feb11_chip_info = { + .name = "mcp47feb11", + .phys_channels = 1, + .resolution = 10, + .have_ext_vref2 = false, + .have_eeprom = true, +}; + +static const struct mcp47feb02_features mcp47feb21_chip_info = { + .name = "mcp47feb21", + .phys_channels = 1, + .resolution = 12, + .have_ext_vref2 = false, + .have_eeprom = true, +}; + +static const struct mcp47feb02_features mcp47feb02_chip_info = { + .name = "mcp47feb02", + .phys_channels = 2, + .resolution = 8, + .have_ext_vref2 = false, + .have_eeprom = true, +}; + +static const struct mcp47feb02_features mcp47feb12_chip_info = { + .name = "mcp47feb12", + .phys_channels = 2, + .resolution = 10, + .have_ext_vref2 = false, + .have_eeprom = true, +}; + +static const struct mcp47feb02_features mcp47feb22_chip_info = { + .name = "mcp47feb22", + .phys_channels = 2, + .resolution = 12, + .have_ext_vref2 = false, + .have_eeprom = true, +}; + +static const struct mcp47feb02_features mcp47feb04_chip_info = { + .name = "mcp47feb04", + .phys_channels = 4, + .resolution = 8, + .have_ext_vref2 = true, + .have_eeprom = true, +}; + +static const struct mcp47feb02_features mcp47feb14_chip_info = { + .name = "mcp47feb14", + .phys_channels = 4, + .resolution = 10, + .have_ext_vref2 = true, + .have_eeprom = true, +}; + +static const struct mcp47feb02_features mcp47feb24_chip_info = { + .name = "mcp47feb24", + .phys_channels = 4, + .resolution = 12, + .have_ext_vref2 = true, + .have_eeprom = true, +}; + +static const struct mcp47feb02_features mcp47feb08_chip_info = { + .name = "mcp47feb08", + .phys_channels = 8, + .resolution = 8, + .have_ext_vref2 = true, + .have_eeprom = true, +}; + +static const struct mcp47feb02_features mcp47feb18_chip_info = { + .name = "mcp47feb18", + .phys_channels = 8, + .resolution = 10, + .have_ext_vref2 = true, + .have_eeprom = true, +}; + +static const struct mcp47feb02_features mcp47feb28_chip_info = { + .name = "mcp47feb28", + .phys_channels = 8, + .resolution = 12, + .have_ext_vref2 = true, + .have_eeprom = true, +}; + +static const struct mcp47feb02_features mcp47fvb01_chip_info = { + .name = "mcp47fvb01", + .phys_channels = 1, + .resolution = 8, + .have_ext_vref2 = false, + .have_eeprom = false, +}; + +static const struct mcp47feb02_features mcp47fvb11_chip_info = { + .name = "mcp47fvb11", + .phys_channels = 1, + .resolution = 10, + .have_ext_vref2 = false, + .have_eeprom = false, +}; + +static const struct mcp47feb02_features mcp47fvb21_chip_info = { + .name = "mcp47fvb21", + .phys_channels = 1, + .resolution = 12, + .have_ext_vref2 = false, + .have_eeprom = false, +}; + +static const struct mcp47feb02_features mcp47fvb02_chip_info = { + .name = "mcp47fvb02", + .phys_channels = 2, + .resolution = 8, + .have_ext_vref2 = false, + .have_eeprom = false, +}; + +static const struct mcp47feb02_features mcp47fvb12_chip_info = { + .name = "mcp47fvb12", + .phys_channels = 2, + .resolution = 8, + .have_ext_vref2 = false, + .have_eeprom = false, +}; + +static const struct mcp47feb02_features mcp47fvb22_chip_info = { + .name = "mcp47fvb22", + .phys_channels = 2, + .resolution = 12, + .have_ext_vref2 = false, + .have_eeprom = false, +}; + +static const struct mcp47feb02_features mcp47fvb04_chip_info = { + .name = "mcp47fvb04", + .phys_channels = 4, + .resolution = 8, + .have_ext_vref2 = true, + .have_eeprom = false, +}; + +static const struct mcp47feb02_features mcp47fvb14_chip_info = { + .name = "mcp47fvb14", + .phys_channels = 4, + .resolution = 10, + .have_ext_vref2 = true, + .have_eeprom = false, +}; + +static const struct mcp47feb02_features mcp47fvb24_chip_info = { + .name = "mcp47fvb24", + .phys_channels = 4, + .resolution = 12, + .have_ext_vref2 = true, + .have_eeprom = false, +}; + +static const struct mcp47feb02_features mcp47fvb08_chip_info = { + .name = "mcp47fvb08", + .phys_channels = 8, + .resolution = 8, + .have_ext_vref2 = true, + .have_eeprom = false, +}; + +static const struct mcp47feb02_features mcp47fvb18_chip_info = { + .name = "mcp47fvb18", + .phys_channels = 8, + .resolution = 10, + .have_ext_vref2 = true, + .have_eeprom = false, +}; + +static const struct mcp47feb02_features mcp47fvb28_chip_info = { + .name = "mcp47fvb28", + .phys_channels = 4, + .resolution = 8, + .have_ext_vref2 = true, + .have_eeprom = false, +}; + +/** + * struct mcp47feb02_channel_data - channel configuration + * @ref_mode: chosen voltage for reference + * @powerdown_mode: selected power-down mode + * @use_2x_gain: output driver gain control + * @powerdown: is false if the channel is in normal operation mode + * @dac_data: read dac value + */ +struct mcp47feb02_channel_data { + enum vref_mode ref_mode; + u8 powerdown_mode; + bool use_2x_gain; + bool powerdown; + u16 dac_data; +}; + +/** + * struct mcp47feb02_data - chip configuration + * @chdata: options configured for each channel on the device + * @scale: scales set on channels that are based on Vref/Vref0 + * @scale_1: scales set on channels that are based on Vref1 + * @labels: table with channels labels + * @active_channels_mask: enabled channels + * @client: the i2c-client attached to the device + * @regmap: regmap for directly accessing device register + * @vref1_buffered: Vref1 buffer is enabled + * @vref_buffered: Vref/Vref0 buffer is enabled + * @phys_channels: physical channels on the device + * @lock: prevents concurrent reads/writes + * @use_vref1: vref1-supply is defined + * @use_vref: vref-supply is defined + * @have_eeprom: does the hardware have an internal eeprom? + */ +struct mcp47feb02_data { + struct mcp47feb02_channel_data chdata[MCP47FEBXX_MAX_CH]; + int scale[MCP47FEB02_MAX_VALS_SCALES_CH]; + int scale_1[MCP47FEB02_MAX_VALS_SCALES_CH]; + const char *labels[MCP47FEBXX_MAX_CH]; + unsigned long active_channels_mask; + struct i2c_client *client; + struct regmap *regmap; + bool vref1_buffered; + bool vref_buffered; + u16 phys_channels; + struct mutex lock; /* synchronize access to driver's state members */ + bool use_vref1; + bool use_vref; + bool have_eeprom; +}; + +static const struct regmap_range mcp47feb02_readable_ranges[] = { + regmap_reg_range(READ_OP(MCP47FEB02_DAC0_REG_ADDR), + READ_OP(MCP47FEB02_WIPERLOCK_STATUS_REG_ADDR)), + regmap_reg_range(READ_OP(MCP47FEB02_NV_DAC0_REG_ADDR), + READ_OP(MCP47FEB02_NV_GAIN_I2C_SLAVE_REG_ADDR)), +}; + +static const struct regmap_range mcp47feb02_writable_ranges[] = { + regmap_reg_range(WRITE_OP(MCP47FEB02_DAC0_REG_ADDR), + WRITE_OP(MCP47FEB02_WIPERLOCK_STATUS_REG_ADDR)), + regmap_reg_range(WRITE_OP(MCP47FEB02_NV_DAC0_REG_ADDR), + WRITE_OP(MCP47FEB02_NV_GAIN_I2C_SLAVE_REG_ADDR)), +}; + +static const struct regmap_range mcp47feb02_volatile_ranges[] = { + regmap_reg_range(WRITE_OP(MCP47FEB02_DAC0_REG_ADDR), + WRITE_OP(MCP47FEB02_WIPERLOCK_STATUS_REG_ADDR)), + regmap_reg_range(WRITE_OP(MCP47FEB02_NV_DAC0_REG_ADDR), + WRITE_OP(MCP47FEB02_NV_GAIN_I2C_SLAVE_REG_ADDR)), + regmap_reg_range(READ_OP(MCP47FEB02_DAC0_REG_ADDR), + READ_OP(MCP47FEB02_WIPERLOCK_STATUS_REG_ADDR)), + regmap_reg_range(READ_OP(MCP47FEB02_NV_DAC0_REG_ADDR), + READ_OP(MCP47FEB02_NV_GAIN_I2C_SLAVE_REG_ADDR)), +}; + +static const struct regmap_access_table mcp47feb02_readable_table = { + .yes_ranges = mcp47feb02_readable_ranges, + .n_yes_ranges = ARRAY_SIZE(mcp47feb02_readable_ranges), +}; + +static const struct regmap_access_table mcp47feb02_writable_table = { + .yes_ranges = mcp47feb02_writable_ranges, + .n_yes_ranges = ARRAY_SIZE(mcp47feb02_writable_ranges), +}; + +static const struct regmap_access_table mcp47feb02_volatile_table = { + .yes_ranges = mcp47feb02_volatile_ranges, + .n_yes_ranges = ARRAY_SIZE(mcp47feb02_volatile_ranges), +}; + +static const struct regmap_config mcp47feb02_regmap_config = { + .name = "mcp47feb02_regmap", + .reg_bits = 8, + .val_bits = 16, + .rd_table = &mcp47feb02_readable_table, + .wr_table = &mcp47feb02_writable_table, + .volatile_table = &mcp47feb02_volatile_table, + .max_register = READ_OP(MCP47FEB02_NV_GAIN_I2C_SLAVE_REG_ADDR), + .cache_type = REGCACHE_RBTREE, +}; + +/* For devices that doesn't have nonvolatile memory */ +static const struct regmap_range mcp47fvb02_readable_ranges[] = { + regmap_reg_range(READ_OP(MCP47FEB02_DAC0_REG_ADDR), + READ_OP(MCP47FEB02_WIPERLOCK_STATUS_REG_ADDR)), +}; + +static const struct regmap_range mcp47fvb02_writable_ranges[] = { + regmap_reg_range(WRITE_OP(MCP47FEB02_DAC0_REG_ADDR), + WRITE_OP(MCP47FEB02_WIPERLOCK_STATUS_REG_ADDR)), +}; + +static const struct regmap_range mcp47fvb02_volatile_ranges[] = { + regmap_reg_range(WRITE_OP(MCP47FEB02_DAC0_REG_ADDR), + WRITE_OP(MCP47FEB02_WIPERLOCK_STATUS_REG_ADDR)), + regmap_reg_range(READ_OP(MCP47FEB02_DAC0_REG_ADDR), + READ_OP(MCP47FEB02_WIPERLOCK_STATUS_REG_ADDR)), +}; + +static const struct regmap_access_table mcp47fvb02_readable_table = { + .yes_ranges = mcp47fvb02_readable_ranges, + .n_yes_ranges = ARRAY_SIZE(mcp47fvb02_readable_ranges), +}; + +static const struct regmap_access_table mcp47fvb02_writable_table = { + .yes_ranges = mcp47fvb02_writable_ranges, + .n_yes_ranges = ARRAY_SIZE(mcp47fvb02_writable_ranges), +}; + +static const struct regmap_access_table mcp47fvb02_volatile_table = { + .yes_ranges = mcp47fvb02_volatile_ranges, + .n_yes_ranges = ARRAY_SIZE(mcp47fvb02_volatile_ranges), +}; + +static const struct regmap_config mcp47fvb02_regmap_config = { + .name = "mcp47fvb02_regmap", + .reg_bits = 8, + .val_bits = 16, + .rd_table = &mcp47fvb02_readable_table, + .wr_table = &mcp47fvb02_writable_table, + .volatile_table = &mcp47fvb02_volatile_table, + .max_register = READ_OP(MCP47FEB02_WIPERLOCK_STATUS_REG_ADDR), + .cache_type = REGCACHE_RBTREE, +}; + +static int mcp47feb02_write_to_register(struct regmap *regmap, unsigned int reg, + int channel, int tmp_val) +{ + int ret, val; + + ret = regmap_read(regmap, CMD_FORMAT(reg, READ_CMD), &val); + if (ret) + return ret; + + if (reg == MCP47FEB02_GAIN_STATUS_REG_ADDR) { + switch (channel) { + case 0: + FIELD_MODIFY(G_0, &val, tmp_val); + break; + case 1: + FIELD_MODIFY(G_1, &val, tmp_val); + break; + case 2: + FIELD_MODIFY(G_2, &val, tmp_val); + break; + case 3: + FIELD_MODIFY(G_3, &val, tmp_val); + break; + case 4: + FIELD_MODIFY(G_4, &val, tmp_val); + break; + case 5: + FIELD_MODIFY(G_5, &val, tmp_val); + break; + case 6: + FIELD_MODIFY(G_6, &val, tmp_val); + break; + case 7: + FIELD_MODIFY(G_7, &val, tmp_val); + break; + default: + FIELD_MODIFY(G_0, &val, tmp_val); + break; + } + } else { + switch (channel) { + case 0: + FIELD_MODIFY(CH_0, &val, tmp_val); + break; + case 1: + FIELD_MODIFY(CH_1, &val, tmp_val); + break; + case 2: + FIELD_MODIFY(CH_2, &val, tmp_val); + break; + case 3: + FIELD_MODIFY(CH_3, &val, tmp_val); + break; + case 4: + FIELD_MODIFY(CH_4, &val, tmp_val); + break; + case 5: + FIELD_MODIFY(CH_5, &val, tmp_val); + break; + case 6: + FIELD_MODIFY(CH_6, &val, tmp_val); + break; + case 7: + FIELD_MODIFY(CH_7, &val, tmp_val); + break; + default: + FIELD_MODIFY(CH_0, &val, tmp_val); + break; + } + } + + ret = regmap_write(regmap, CMD_FORMAT(reg, WRITE_CMD), val); + + return ret; +} + +static int mcp47feb02_write_to_eeprom(struct regmap *regmap, unsigned int reg, + unsigned int val) +{ + int eewa_val, ret; + + /* + * wait till the currently occurring EEPROM Write Cycle is completed. + * Only serial commands to the volatile memory are allowed. + */ + ret = regmap_read_poll_timeout(regmap, + CMD_FORMAT(MCP47FEB02_GAIN_STATUS_REG_ADDR, READ_CMD), + eewa_val, !(eewa_val & MCP47FEB02_GAIN_STATUS_EEWA_MASK), + MCP47FEB02_DELAY_1_MS, MCP47FEB02_DELAY_1_MS * 5); + if (ret) + return ret; + + ret = regmap_write(regmap, CMD_FORMAT(reg, WRITE_CMD), val); + + return ret; +} + +static ssize_t mcp47feb02_store_eeprom(struct device *dev, struct device_attribute *attr, + const char *buf, size_t len) +{ + struct mcp47feb02_data *data = iio_priv(dev_to_iio_dev(dev)); + int ret, i, val, val1; + bool state; + + ret = kstrtobool(buf, &state); + if (ret < 0) + return ret; + + if (!state) + return 0; + + if (!data->have_eeprom) { + dev_err(dev, "Device has no eeprom memory.\n"); + return -EINVAL; + } + + /* + * Verify DAC Wiper and DAC Configuratioin are unlocked. If both are disabled, + * writing to EEPROM is available. + */ + ret = regmap_read(data->regmap, CMD_FORMAT(MCP47FEB02_WIPERLOCK_STATUS_REG_ADDR, READ_CMD), + &val); + if (ret) + return ret; + + if (val) { + dev_err(dev, "DAC Wiper and DAC Configuration not are unlocked.\n"); + return -EINVAL; + } + + for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) { + ret = mcp47feb02_write_to_eeprom(data->regmap, MCP47FEB02_NV_DAC0_REG_ADDR + i, + data->chdata[i].dac_data); + if (ret) + return ret; + } + + ret = regmap_read(data->regmap, CMD_FORMAT(MCP47FEB02_VREF_REG_ADDR, READ_CMD), &val); + if (ret) + return ret; + + ret = mcp47feb02_write_to_eeprom(data->regmap, MCP47FEB02_NV_VREF_REG_ADDR, val); + if (ret) + return ret; + + ret = regmap_read(data->regmap, CMD_FORMAT(MCP47FEB02_POWER_DOWN_REG_ADDR, READ_CMD), &val); + if (ret) + return ret; + + ret = mcp47feb02_write_to_eeprom(data->regmap, MCP47FEB02_NV_POWER_DOWN_REG_ADDR, val); + if (ret) + return ret; + + ret = regmap_read(data->regmap, + CMD_FORMAT(MCP47FEB02_NV_GAIN_I2C_SLAVE_REG_ADDR, READ_CMD), &val); + if (ret) + return ret; + + ret = regmap_read(data->regmap, + CMD_FORMAT(MCP47FEB02_GAIN_STATUS_REG_ADDR, READ_CMD), &val1); + if (ret) + return ret; + + ret = mcp47feb02_write_to_eeprom(data->regmap, MCP47FEB02_NV_GAIN_I2C_SLAVE_REG_ADDR, + (val1 & MCP47FEB02_VOLATILE_GAIN_MASK) | + (val & MCP47FEB02_NV_I2C_SLAVE_ADDR_MASK)); + if (ret) + return ret; + + return len; +} + +static IIO_DEVICE_ATTR(store_eeprom, 0200, NULL, mcp47feb02_store_eeprom, 0); +static struct attribute *mcp47feb02_attributes[] = { + &iio_dev_attr_store_eeprom.dev_attr.attr, + NULL, +}; + +static int mcp47feb02_suspend(struct device *dev) +{ + struct iio_dev *indio_dev = dev_get_drvdata(dev); + struct mcp47feb02_data *data = iio_priv(indio_dev); + int ret, ch; + + for_each_set_bit(ch, &data->active_channels_mask, data->phys_channels) { + data->chdata[ch].powerdown = true; + ret = mcp47feb02_write_to_register(data->regmap, MCP47FEB02_POWER_DOWN_REG_ADDR, + ch, data->chdata[ch].powerdown_mode + 1); + if (ret) + return ret; + + ret = regmap_write(data->regmap, CMD_FORMAT(ch, WRITE_CMD), + data->chdata[ch].dac_data); + if (ret) + return ret; + } + + return 0; +} + +static int mcp47feb02_resume(struct device *dev) +{ + struct iio_dev *indio_dev = dev_get_drvdata(dev); + struct mcp47feb02_data *data = iio_priv(indio_dev); + int ch, ret; + + for_each_set_bit(ch, &data->active_channels_mask, data->phys_channels) { + data->chdata[ch].powerdown = false; + + ret = regmap_write(data->regmap, CMD_FORMAT(ch, WRITE_CMD), + data->chdata[ch].dac_data); + if (ret) + return ret; + + ret = mcp47feb02_write_to_register(data->regmap, MCP47FEB02_VREF_REG_ADDR, + ch, data->chdata[ch].ref_mode); + if (ret) + return ret; + + ret = mcp47feb02_write_to_register(data->regmap, MCP47FEB02_GAIN_STATUS_REG_ADDR, + ch, data->chdata[ch].use_2x_gain); + if (ret) + return ret; + + ret = mcp47feb02_write_to_register(data->regmap, MCP47FEB02_POWER_DOWN_REG_ADDR, + ch, MCP47FEB02_NORMAL_OPERATION); + if (ret) + return ret; + } + + return 0; +} + +static DEFINE_SIMPLE_DEV_PM_OPS(mcp47feb02_pm_ops, mcp47feb02_suspend, mcp47feb02_resume); + +static const struct attribute_group mcp47feb02_attribute_group = { + .attrs = mcp47feb02_attributes, +}; + +static const char * const mcp47feb02_powerdown_modes[] = { + "1kohm_to_gnd", + "100kohm_to_gnd", + "open_circuit", +}; + +static int mcp47feb02_get_powerdown_mode(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan) +{ + struct mcp47feb02_data *data = iio_priv(indio_dev); + int bit_set; + + bit_set = test_bit(chan->address, &data->active_channels_mask); + if (bit_set) + return data->chdata[chan->address].powerdown_mode; + + return -EINVAL; +} + +static int mcp47feb02_set_powerdown_mode(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan, unsigned int mode) +{ + struct mcp47feb02_data *data = iio_priv(indio_dev); + struct device *dev = &data->client->dev; + int bit_set, ret; + + bit_set = test_bit(chan->address, &data->active_channels_mask); + if (bit_set) { + ret = mcp47feb02_write_to_register(data->regmap, MCP47FEB02_POWER_DOWN_REG_ADDR, + chan->address, mode + 1); + if (ret) + return ret; + + data->chdata[chan->address].powerdown_mode = mode; + + return 0; + } + + dev_err(dev, "Channel %ld not enabled\n", chan->address); + return -EINVAL; +} + +static ssize_t mcp47feb02_read_powerdown(struct iio_dev *indio_dev, uintptr_t private, + const struct iio_chan_spec *chan, char *buf) +{ + struct mcp47feb02_data *data = iio_priv(indio_dev); + int bit_set; + + bit_set = test_bit(chan->address, &data->active_channels_mask); + if (bit_set) + return sysfs_emit(buf, "%d\n", data->chdata[chan->address].powerdown); + + dev_err(&data->client->dev, "Channel %ld not enabled\n", chan->address); + return -EINVAL; +} + +static ssize_t mcp47feb02_write_powerdown(struct iio_dev *indio_dev, uintptr_t private, + const struct iio_chan_spec *chan, const char *buf, + size_t len) +{ + struct mcp47feb02_data *data = iio_priv(indio_dev); + struct device *dev = &data->client->dev; + int bit_set, ret; + bool state; + + ret = kstrtobool(buf, &state); + if (ret) + return ret; + + bit_set = test_bit(chan->address, &data->active_channels_mask); + if (bit_set) + data->chdata[chan->address].powerdown = state; + else + dev_err(dev, "Channel %ld not enabled\n", chan->address); + + return len; +} + +static const struct iio_enum mcp47febxx_powerdown_mode_enum = { + .items = mcp47feb02_powerdown_modes, + .num_items = ARRAY_SIZE(mcp47feb02_powerdown_modes), + .get = mcp47feb02_get_powerdown_mode, + .set = mcp47feb02_set_powerdown_mode, +}; + +static const struct iio_chan_spec_ext_info mcp47feb02_ext_info[] = { + { + .name = "powerdown", + .read = mcp47feb02_read_powerdown, + .write = mcp47feb02_write_powerdown, + .shared = IIO_SEPARATE, + }, + IIO_ENUM("powerdown_mode", IIO_SEPARATE, &mcp47febxx_powerdown_mode_enum), + IIO_ENUM_AVAILABLE("powerdown_mode", IIO_SHARED_BY_TYPE, &mcp47febxx_powerdown_mode_enum), + { }, +}; + +static const struct iio_chan_spec mcp47febxx_ch_template = { + .type = IIO_VOLTAGE, + .output = 1, + .indexed = 1, + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | + BIT(IIO_CHAN_INFO_SCALE), + .info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE), + .ext_info = mcp47feb02_ext_info, +}; + +static void mcp47feb02_init_scale(const struct mcp47feb02_features *info, + enum mcp47feb02_scale scale, int vref_mv, + int scale_avail[]) +{ + int value_micro, value_int; + s64 tmp; + + tmp = (s64)vref_mv * 1000000LL >> info->resolution; + value_int = div_s64_rem(tmp, 1000000LL, &value_micro); + scale_avail[scale * 2] = value_int; + scale_avail[scale * 2 + 1] = value_micro; +} + +static int mcp47feb02_init_scales_avail(const struct mcp47feb02_features *info, + struct mcp47feb02_data *data, int vdd_mv, int vref_mv, + int vref1_mv) +{ + struct device *dev = &data->client->dev; + int tmp_vref; + + if (data->use_vref && vref_mv <= 0) { + dev_err(dev, "use_vref set but vref_mv invalid\n"); + return -EINVAL; + } + + mcp47feb02_init_scale(info, MCP47FEB02_SCALE_VDD, vdd_mv, data->scale); + + if (data->use_vref) + tmp_vref = vref_mv; + else + tmp_vref = MCP47FEB02_INTERNAL_BAND_GAP_MV; + + mcp47feb02_init_scale(info, MCP47FEB02_SCALE_GAIN_X1, tmp_vref, data->scale); + mcp47feb02_init_scale(info, MCP47FEB02_SCALE_GAIN_X2, tmp_vref * 2, data->scale); + + if (data->phys_channels >= 4) { + mcp47feb02_init_scale(info, MCP47FEB02_SCALE_VDD, vdd_mv, data->scale_1); + + if (data->use_vref1 && vref1_mv <= 0) { + dev_err(dev, "use_vref1 set but vref1_mv invalid\n"); + return -EINVAL; + } + + if (data->use_vref1) + tmp_vref = vref1_mv; + else + tmp_vref = MCP47FEB02_INTERNAL_BAND_GAP_MV; + + mcp47feb02_init_scale(info, MCP47FEB02_SCALE_GAIN_X1, tmp_vref, + data->scale_1); + mcp47feb02_init_scale(info, MCP47FEB02_SCALE_GAIN_X2, tmp_vref * 2, + data->scale_1); + } + + return 0; +} + +static int mcp47feb02_read_avail(struct iio_dev *indio_dev, struct iio_chan_spec const *chan, + const int **vals, int *type, int *length, long info) +{ + struct mcp47feb02_data *data = iio_priv(indio_dev); + + switch (info) { + case IIO_CHAN_INFO_SCALE: + switch (chan->type) { + case IIO_VOLTAGE: + if (data->phys_channels >= 4 && (chan->address % 2)) + *vals = data->scale_1; + else + *vals = data->scale; + + *length = MCP47FEB02_MAX_VALS_SCALES_CH; + *type = IIO_VAL_INT_PLUS_MICRO; + return IIO_AVAIL_LIST; + default: + return -EINVAL; + } + default: + return -EINVAL; + } +} + +static void mcp47feb02_get_scale_avail(enum mcp47feb02_scale scale, struct mcp47feb02_data *data, + int *val, int *val2, int channel) +{ + if (data->phys_channels >= 4 && (channel % 2)) { + *val = data->scale_1[scale * 2]; + *val2 = data->scale_1[scale * 2 + 1]; + } else { + *val = data->scale[scale * 2]; + *val2 = data->scale[scale * 2 + 1]; + } +} + +static void mcp47feb02_get_scale(int channel, struct mcp47feb02_data *data, + int *val, int *val2) +{ + if (data->chdata[channel].ref_mode == MCP47FEB02_VREF_VDD) + mcp47feb02_get_scale_avail(MCP47FEB02_SCALE_VDD, data, val, + val2, channel); + else if (data->chdata[channel].use_2x_gain) + mcp47feb02_get_scale_avail(MCP47FEB02_SCALE_GAIN_X2, data, val, + val2, channel); + else + mcp47feb02_get_scale_avail(MCP47FEB02_SCALE_GAIN_X1, data, val, + val2, channel); +} + +static int mcp47feb02_check_scale(struct mcp47feb02_data *data, int val, int val2, + int scale[]) +{ + for (int i = 0; i < MCP47FEB02_MAX_SCALES_CH; i++) { + if (scale[i * 2] == val && scale[i * 2 + 1] == val2) + return i; + } + + return -EINVAL; +} + +static int mcp47feb02_ch_scale(int num_channels, struct mcp47feb02_data *data, + int channel, int scale) +{ + int tmp_val, ret; + + if (scale == MCP47FEB02_SCALE_VDD) { + tmp_val = MCP47FEB02_VREF_VDD; + } else if (data->phys_channels >= 4 && (channel % 2)) { + if (data->use_vref1) { + if (data->vref1_buffered) + tmp_val = MCP47FEB02_EXTERNAL_VREF_BUFFERED; + else + tmp_val = MCP47FEB02_EXTERNAL_VREF_UNBUFFERED; + } else { + tmp_val = MCP47FEB02_INTERNAL_BAND_GAP; + } + } else { + if (data->use_vref) { + if (data->vref_buffered) + tmp_val = MCP47FEB02_EXTERNAL_VREF_BUFFERED; + else + tmp_val = MCP47FEB02_EXTERNAL_VREF_UNBUFFERED; + } else { + tmp_val = MCP47FEB02_INTERNAL_BAND_GAP; + } + } + + ret = mcp47feb02_write_to_register(data->regmap, MCP47FEB02_VREF_REG_ADDR, + channel, tmp_val); + if (ret) + return ret; + + data->chdata[channel].ref_mode = tmp_val; + + return 0; +} + +static int mcp47feb02_set_scale(struct mcp47feb02_data *data, int channel, int scale) +{ + int tmp_val, ret; + + mcp47feb02_ch_scale(data->phys_channels, data, channel, scale); + + if (scale == MCP47FEB02_SCALE_GAIN_X2) + tmp_val = MCP47FEB02_GAIN_X2; + else + tmp_val = MCP47FEB02_GAIN_X1; + + ret = mcp47feb02_write_to_register(data->regmap, MCP47FEB02_GAIN_STATUS_REG_ADDR, + channel, tmp_val); + if (ret) + return ret; + + data->chdata[channel].use_2x_gain = tmp_val; + + return 0; +} + +static int mcp47feb02_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan, + int *val, int *val2, long mask) +{ + struct mcp47feb02_data *data = iio_priv(indio_dev); + int ret; + + switch (mask) { + case IIO_CHAN_INFO_RAW: + ret = regmap_read(data->regmap, CMD_FORMAT(chan->address, READ_CMD), val); + if (ret) + return ret; + + return IIO_VAL_INT; + case IIO_CHAN_INFO_SCALE: + mcp47feb02_get_scale(chan->address, data, val, val2); + return IIO_VAL_INT_PLUS_MICRO; + default: + return -EINVAL; + } +} + +static int mcp47feb02_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan, + int val, int val2, long mask) +{ + struct mcp47feb02_data *data = iio_priv(indio_dev); + int ret; + + switch (mask) { + case IIO_CHAN_INFO_RAW: + ret = regmap_write(data->regmap, CMD_FORMAT(chan->address, WRITE_CMD), val); + data->chdata[chan->address].dac_data = val; + return ret; + case IIO_CHAN_INFO_SCALE: + if (data->phys_channels >= 4 && (chan->address % 2)) + ret = mcp47feb02_check_scale(data, val, val2, data->scale_1); + else + ret = mcp47feb02_check_scale(data, val, val2, data->scale); + + if (ret < 0) + return ret; + + return mcp47feb02_set_scale(data, chan->address, ret); + default: + return -EINVAL; + } +} + +static const struct iio_info mcp47feb02_info = { + .read_raw = mcp47feb02_read_raw, + .write_raw = mcp47feb02_write_raw, + .read_avail = &mcp47feb02_read_avail, + .attrs = &mcp47feb02_attribute_group, +}; + +static const struct iio_info mcp47fvb02_info = { + .read_raw = mcp47feb02_read_raw, + .write_raw = mcp47feb02_write_raw, + .read_avail = &mcp47feb02_read_avail, + .attrs = &mcp47feb02_attribute_group, +}; + +static int mcp47feb02_parse_fw(struct iio_dev *indio_dev, const struct mcp47feb02_features *info) +{ + struct iio_chan_spec chanspec = mcp47febxx_ch_template; + struct mcp47feb02_data *data = iio_priv(indio_dev); + struct device *dev = &data->client->dev; + struct iio_chan_spec *channels; + u32 reg, num_channels; + int chan_idx = 0; + + num_channels = device_get_child_node_count(dev); + if (num_channels > info->phys_channels) + return dev_err_probe(dev, -EINVAL, "More channels than the chip supports\n"); + + channels = devm_kcalloc(dev, num_channels, sizeof(*channels), GFP_KERNEL); + if (!channels) + return -ENOMEM; + + device_for_each_child_node_scoped(dev, child) { + fwnode_property_read_u32(child, "reg", ®); + + if (reg >= info->phys_channels) + return dev_err_probe(dev, -EINVAL, + "The index of the channels does not match the chip\n"); + + set_bit(reg, &data->active_channels_mask); + + if (fwnode_property_present(child, "label")) + fwnode_property_read_string(child, "label", + (const char **)&data->labels[reg]); + + chanspec.address = reg; + chanspec.channel = reg; + channels[chan_idx] = chanspec; + chan_idx++; + } + + indio_dev->num_channels = num_channels; + indio_dev->channels = channels; + indio_dev->modes = INDIO_DIRECT_MODE; + data->phys_channels = info->phys_channels; + + /* + * check if vref-supply, vref1-supply, microchip,vref-buffered and + * microchip,vref1-buffered are defined in the devicetree + */ + data->use_vref = device_property_present(dev, "vref-supply"); + data->use_vref1 = device_property_present(dev, "vref1-supply"); + data->vref_buffered = device_property_read_bool(dev, "microchip,vref-buffered"); + data->vref1_buffered = device_property_read_bool(dev, "microchip,vref1-buffered"); + + return 0; +} + +static int mcp47feb02_probe(struct i2c_client *client) +{ + int err, ret, vdd_mv, vref_mv, vref1_mv, i, tmp_vref, vref_ch, gain_ch; + const struct i2c_device_id *id = i2c_client_get_device_id(client); + const struct mcp47feb02_features *info; + enum vref_mode ref_mode, ref_mode1; + struct device *dev = &client->dev; + struct mcp47feb02_data *data; + struct iio_dev *indio_dev; + + indio_dev = devm_iio_device_alloc(dev, sizeof(*data)); + if (!indio_dev) + return -ENOMEM; + + data = iio_priv(indio_dev); + i2c_set_clientdata(client, indio_dev); + data->client = client; + info = i2c_get_match_data(client); + + if (info->have_eeprom) { + data->regmap = devm_regmap_init_i2c(client, &mcp47feb02_regmap_config); + data->have_eeprom = true; + } else { + data->regmap = devm_regmap_init_i2c(client, &mcp47fvb02_regmap_config); + data->have_eeprom = false; + } + + if (IS_ERR(data->regmap)) + dev_err_probe(dev, PTR_ERR(data->regmap), "Error initializing i2c regmap\n"); + + err = mcp47feb02_parse_fw(indio_dev, info); + if (err) + return dev_err_probe(dev, err, "Error parsing devicetree data\n"); + + if (!info->have_ext_vref2 && data->use_vref1) + return dev_err_probe(dev, -EINVAL, + "Second External reference is unavailable on %s\n", + info->name); + + ret = regmap_read(data->regmap, CMD_FORMAT(MCP47FEB02_VREF_REG_ADDR, READ_CMD), &vref_ch); + if (ret) + return ret; + + ret = regmap_read(data->regmap, CMD_FORMAT(MCP47FEB02_GAIN_STATUS_REG_ADDR, READ_CMD), + &gain_ch); + if (ret) + return ret; + + gain_ch = gain_ch >> 8; + + /* + * Values stored in the nonvolatile memory will be transferred to the volatile registers + * at startup. For safety reasons, the user receives a warning if startup values + * do not match the ones from current devicetree configuration. + * Nonvolatile memory can be written at any time + */ + for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) { + /* VDD can be set as Vref only with Gain x1 */ + if ((vref_ch & 0x03) == MCP47FEB02_VREF_VDD && + (gain_ch & 0x01) == MCP47FEB02_GAIN_X2) { + dev_info(dev, "vdd can be used only with gain x1\n"); + ret = mcp47feb02_write_to_register(data->regmap, + MCP47FEB02_GAIN_STATUS_REG_ADDR, + i, MCP47FEB02_GAIN_X1); + if (ret) + return ret; + + data->chdata[i].use_2x_gain = MCP47FEB02_GAIN_X1; + } + + if (data->phys_channels >= 4 && (i % 2)) { + if ((vref_ch & 0x03) == MCP47FEB02_EXTERNAL_VREF_BUFFERED && + data->use_vref1 && !data->vref1_buffered) + dev_info(dev, "vref1 is unbuffered\n"); + else if ((vref_ch & 0x03) == MCP47FEB02_EXTERNAL_VREF_UNBUFFERED && + data->use_vref1 && data->vref1_buffered) + dev_info(dev, "vref1 is buffered\n"); + } else { + if ((vref_ch & 0x03) == MCP47FEB02_EXTERNAL_VREF_BUFFERED && + data->use_vref && !data->vref_buffered) + dev_info(dev, "vref is unbuffered\n"); + else if ((vref_ch & 0x03) == MCP47FEB02_EXTERNAL_VREF_UNBUFFERED && + data->use_vref && data->vref_buffered) + dev_info(dev, "vref is buffered\n"); + } + + vref_ch = vref_ch >> 2; + gain_ch = gain_ch >> 1; + } + + if (data->use_vref) + ref_mode = data->vref_buffered ? + MCP47FEB02_EXTERNAL_VREF_BUFFERED : MCP47FEB02_EXTERNAL_VREF_UNBUFFERED; + else + ref_mode = MCP47FEB02_INTERNAL_BAND_GAP; + + if (data->use_vref1) + ref_mode1 = data->vref1_buffered ? + MCP47FEB02_EXTERNAL_VREF_BUFFERED : MCP47FEB02_EXTERNAL_VREF_UNBUFFERED; + + else + ref_mode1 = MCP47FEB02_INTERNAL_BAND_GAP; + + for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) { + if (data->phys_channels >= 4 && (i % 2)) + tmp_vref = ref_mode1; + else + tmp_vref = ref_mode; + + ret = mcp47feb02_write_to_register(data->regmap, MCP47FEB02_VREF_REG_ADDR, + i, tmp_vref); + if (ret) + return ret; + + data->chdata[i].ref_mode = tmp_vref; + } + + indio_dev->name = id->name; + if (info->have_eeprom) + indio_dev->info = &mcp47feb02_info; + else + indio_dev->info = &mcp47fvb02_info; + + ret = devm_mutex_init(dev, &data->lock); + if (ret < 0) + return ret; + + ret = devm_regulator_get_enable_read_voltage(dev, "vdd"); + if (ret < 0) + return ret; + + vdd_mv = ret / 1000; + + if (data->use_vref) { + ret = devm_regulator_get_enable_read_voltage(dev, "vref"); + if (ret < 0) + return ret; + + vref_mv = ret / 1000; + } + + if (data->use_vref1) { + ret = devm_regulator_get_enable_read_voltage(dev, "vref1"); + if (ret < 0) + return ret; + + vref1_mv = ret / 1000; + } + + for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) { + ret = mcp47feb02_init_scales_avail(info, data, vdd_mv, vref_mv, vref1_mv); + if (ret) + dev_err_probe(dev, ret, "failed to init scales for ch i %d\n", i); + } + + err = iio_device_register(indio_dev); + + return err; +} + +static void mcp47feb02_remove(struct i2c_client *client) +{ + struct iio_dev *indio_dev = i2c_get_clientdata(client); + + iio_device_unregister(indio_dev); +} + +static const struct i2c_device_id mcp47feb02_id[] = { + { "mcp47feb01", (kernel_ulong_t)&mcp47feb01_chip_info }, + { "mcp47feb11", (kernel_ulong_t)&mcp47feb11_chip_info }, + { "mcp47feb21", (kernel_ulong_t)&mcp47feb21_chip_info }, + { "mcp47feb02", (kernel_ulong_t)&mcp47feb02_chip_info }, + { "mcp47feb12", (kernel_ulong_t)&mcp47feb12_chip_info }, + { "mcp47feb22", (kernel_ulong_t)&mcp47feb22_chip_info }, + { "mcp47feb04", (kernel_ulong_t)&mcp47feb04_chip_info }, + { "mcp47feb14", (kernel_ulong_t)&mcp47feb14_chip_info }, + { "mcp47feb24", (kernel_ulong_t)&mcp47feb24_chip_info }, + { "mcp47feb08", (kernel_ulong_t)&mcp47feb08_chip_info }, + { "mcp47feb18", (kernel_ulong_t)&mcp47feb18_chip_info }, + { "mcp47feb28", (kernel_ulong_t)&mcp47feb28_chip_info }, + { "mcp47fvb01", (kernel_ulong_t)&mcp47fvb01_chip_info }, + { "mcp47fvb11", (kernel_ulong_t)&mcp47fvb11_chip_info }, + { "mcp47fvb21", (kernel_ulong_t)&mcp47fvb21_chip_info }, + { "mcp47fvb02", (kernel_ulong_t)&mcp47fvb02_chip_info }, + { "mcp47fvb12", (kernel_ulong_t)&mcp47fvb12_chip_info }, + { "mcp47fvb22", (kernel_ulong_t)&mcp47fvb22_chip_info }, + { "mcp47fvb04", (kernel_ulong_t)&mcp47fvb04_chip_info }, + { "mcp47fvb14", (kernel_ulong_t)&mcp47fvb14_chip_info }, + { "mcp47fvb24", (kernel_ulong_t)&mcp47fvb24_chip_info }, + { "mcp47fvb08", (kernel_ulong_t)&mcp47fvb08_chip_info }, + { "mcp47fvb18", (kernel_ulong_t)&mcp47fvb18_chip_info }, + { "mcp47fvb28", (kernel_ulong_t)&mcp47fvb28_chip_info }, + { } +}; +MODULE_DEVICE_TABLE(i2c, mcp47feb02_id); + +static const struct of_device_id mcp47feb02_of_match[] = { + { .compatible = "microchip,mcp47feb01", .data = &mcp47feb01_chip_info }, + { .compatible = "microchip,mcp47feb11", .data = &mcp47feb11_chip_info }, + { .compatible = "microchip,mcp47feb21", .data = &mcp47feb21_chip_info }, + { .compatible = "microchip,mcp47feb02", .data = &mcp47feb02_chip_info }, + { .compatible = "microchip,mcp47feb12", .data = &mcp47feb12_chip_info }, + { .compatible = "microchip,mcp47feb22", .data = &mcp47feb22_chip_info }, + { .compatible = "microchip,mcp47feb04", .data = &mcp47feb04_chip_info }, + { .compatible = "microchip,mcp47feb14", .data = &mcp47feb14_chip_info }, + { .compatible = "microchip,mcp47feb24", .data = &mcp47feb24_chip_info }, + { .compatible = "microchip,mcp47feb08", .data = &mcp47feb08_chip_info }, + { .compatible = "microchip,mcp47feb18", .data = &mcp47feb18_chip_info }, + { .compatible = "microchip,mcp47feb28", .data = &mcp47feb28_chip_info }, + { .compatible = "microchip,mcp47fvb01", .data = &mcp47fvb01_chip_info }, + { .compatible = "microchip,mcp47fvb11", .data = &mcp47fvb11_chip_info }, + { .compatible = "microchip,mcp47fvb21", .data = &mcp47fvb21_chip_info }, + { .compatible = "microchip,mcp47fvb02", .data = &mcp47fvb02_chip_info }, + { .compatible = "microchip,mcp47fvb12", .data = &mcp47fvb12_chip_info }, + { .compatible = "microchip,mcp47fvb22", .data = &mcp47fvb22_chip_info }, + { .compatible = "microchip,mcp47fvb04", .data = &mcp47fvb04_chip_info }, + { .compatible = "microchip,mcp47fvb14", .data = &mcp47fvb14_chip_info }, + { .compatible = "microchip,mcp47fvb24", .data = &mcp47fvb24_chip_info }, + { .compatible = "microchip,mcp47fvb08", .data = &mcp47fvb08_chip_info }, + { .compatible = "microchip,mcp47fvb18", .data = &mcp47fvb18_chip_info }, + { .compatible = "microchip,mcp47fvb28", .data = &mcp47fvb28_chip_info }, + { } +}; +MODULE_DEVICE_TABLE(of, mcp47feb02_of_match); + +static struct i2c_driver mcp47feb02_driver = { + .driver = { + .name = "mcp47feb02", + .of_match_table = mcp47feb02_of_match, + .pm = pm_sleep_ptr(&mcp47feb02_pm_ops), + }, + .probe = mcp47feb02_probe, + .remove = mcp47feb02_remove, + .id_table = mcp47feb02_id, +}; +module_i2c_driver(mcp47feb02_driver); + +MODULE_AUTHOR("Ariana Lazar <ariana.lazar@microchip.com>"); +MODULE_DESCRIPTION("IIO driver for MCP47FEB02 Multi-Channel DAC with I2C interface"); +MODULE_LICENSE("GPL"); -- 2.43.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] iio: dac: adding support for Microchip MCP47FEB02 2025-09-22 11:30 ` [PATCH 2/2] " Ariana Lazar @ 2025-09-22 20:10 ` Nuno Sá 2025-09-22 22:15 ` David Lechner 2025-09-26 15:38 ` Dan Carpenter 2025-09-27 17:53 ` Jonathan Cameron 2 siblings, 1 reply; 14+ messages in thread From: Nuno Sá @ 2025-09-22 20:10 UTC (permalink / raw) To: Ariana Lazar, Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-iio, devicetree, linux-kernel Hi Ariana, Thanks for your patches. Some initial comments from me... On Mon, 2025-09-22 at 14:30 +0300, Ariana Lazar wrote: > This is the iio driver for Microchip MCP47F(E/V)B(0/1/2)1, MCP47F(E/V)B(0/1/2)2, > MCP47F(E/V)B(0/1/2)4 and MCP47F(E/V)B(0/1/2)8 series of buffered voltage output > Digital-to-Analog Converters with nonvolatile or volatile memory and an I2C > Interface. > > The families support up to 8 output channels. > > The devices can be 8-bit, 10-bit and 12-bit. > > Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com> > --- > MAINTAINERS | 1 + > drivers/iio/dac/Kconfig | 16 + > drivers/iio/dac/Makefile | 1 + > drivers/iio/dac/mcp47feb02.c | 1347 ++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 1365 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index > 6f51890cfc3081bc49c08fddc8af526c1ecc8d72..0f97f90ac2f492895d27da86d831df83cb402516 > 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -14943,6 +14943,7 @@ M: Ariana Lazar <ariana.lazar@microchip.com> > L: linux-iio@vger.kernel.org > S: Supported > F: Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml > +F: drivers/iio/dac/mcp47feb02.c > > MCP4821 DAC DRIVER > M: Anshul Dalal <anshulusr@gmail.com> > diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig > index > e0996dc014a3d538ab6b4e0d50ff54ede50f1527..179ce565036e3494e4ce43bb926de31f38b547c4 > 100644 > --- a/drivers/iio/dac/Kconfig > +++ b/drivers/iio/dac/Kconfig > @@ -509,6 +509,22 @@ config MCP4728 > To compile this driver as a module, choose M here: the module > will be called mcp4728. > > +config MCP47FEB02 > + tristate "MCP47F(E/V)B|(0/1/2)(1/2/4/8)DAC driver" > + depends on I2C > + help > + Say yes here if you want to build a driver for the Microchip > + MCP47FEB01, MCP47FEB11, MCP47FEB21, MCP47FEB02, MCP47FEB12, > + MCP47FEB22, MCP47FVB01, MCP47FVB11, MCP47FVB21, MCP47FVB02, > + MCP47FVB12, MCP47FVB02, MCP47FVB12, MCP47FVB22, MCP47FVB04, > + MCP47FVB14, MCP47FVB24, MCP47FVB04, MCP47FVB08, MCP47FVB18, > + MCP47FVB28, MCP47FEB04, MCP47FEB14 and MCP47FEB24 having up to 8 > + channels, 8-bit, 10-bit or 12-bit digital-to-analog converter (DAC) > + with I2C interface. > + > + To compile this driver as a module, choose M here: the module > + will be called mcp47feb02. > + > config MCP4821 > tristate "MCP4801/02/11/12/21/22 DAC driver" > depends on SPI > diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile > index > 3684cd52b7fa9bc0ad9f855323dcbb2e4965c404..d633a6440fc4b9aba7d8b1c209b6dcd05cd982dd > 100644 > --- a/drivers/iio/dac/Makefile > +++ b/drivers/iio/dac/Makefile > @@ -50,6 +50,7 @@ obj-$(CONFIG_MAX5522) += max5522.o > obj-$(CONFIG_MAX5821) += max5821.o > obj-$(CONFIG_MCP4725) += mcp4725.o > obj-$(CONFIG_MCP4728) += mcp4728.o > +obj-$(CONFIG_MCP47FEB02) += mcp47feb02.o > obj-$(CONFIG_MCP4821) += mcp4821.o > obj-$(CONFIG_MCP4922) += mcp4922.o > obj-$(CONFIG_STM32_DAC_CORE) += stm32-dac-core.o > diff --git a/drivers/iio/dac/mcp47feb02.c b/drivers/iio/dac/mcp47feb02.c > new file mode 100644 > index > 0000000000000000000000000000000000000000..c9c2ded78d9c6013e5618e6342ebc8f50e79a31e > --- /dev/null > +++ b/drivers/iio/dac/mcp47feb02.c > @@ -0,0 +1,1347 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * IIO driver for MCP47FEB02 Multi-Channel DAC with I2C interface > + * > + * Copyright (C) 2025 Microchip Technology Inc. and its subsidiaries > + * > + * Author: Ariana Lazar <ariana.lazar@microchip.com> > + * > + * Datasheet for MCP47FEBXX can be found here: > + * > https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005375A.pdf > + * > + * Datasheet for MCP47FVBXX can be found here: > + * > https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005405A.pdf > + * > + * Datasheet for MCP47FXBX4/8 can be found here: > + * > https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/MCP47FXBX48-Data-Sheet-DS200006368A.pdf > + */ > +#include <linux/bits.h> > +#include <linux/bitfield.h> > +#include <linux/delay.h> > +#include <linux/err.h> > +#include <linux/i2c.h> > +#include <linux/iio/iio.h> > +#include <linux/iio/sysfs.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> > + > ... > + > +static int mcp47feb02_write_to_register(struct regmap *regmap, unsigned int reg, > + int channel, int tmp_val) > +{ > + int ret, val; > + > + ret = regmap_read(regmap, CMD_FORMAT(reg, READ_CMD), &val); > + if (ret) > + return ret; > + > + if (reg == MCP47FEB02_GAIN_STATUS_REG_ADDR) { > + switch (channel) { > + case 0: > + FIELD_MODIFY(G_0, &val, tmp_val); > + break; > + case 1: > + FIELD_MODIFY(G_1, &val, tmp_val); > + break; > + case 2: > + FIELD_MODIFY(G_2, &val, tmp_val); > + break; > + case 3: > + FIELD_MODIFY(G_3, &val, tmp_val); > + break; > + case 4: > + FIELD_MODIFY(G_4, &val, tmp_val); > + break; > + case 5: > + FIELD_MODIFY(G_5, &val, tmp_val); > + break; > + case 6: > + FIELD_MODIFY(G_6, &val, tmp_val); > + break; > + case 7: > + FIELD_MODIFY(G_7, &val, tmp_val); > + break; > + default: > + FIELD_MODIFY(G_0, &val, tmp_val); > + break; > + } > + } else { > + switch (channel) { > + case 0: > + FIELD_MODIFY(CH_0, &val, tmp_val); > + break; > + case 1: > + FIELD_MODIFY(CH_1, &val, tmp_val); > + break; > + case 2: > + FIELD_MODIFY(CH_2, &val, tmp_val); > + break; > + case 3: > + FIELD_MODIFY(CH_3, &val, tmp_val); > + break; > + case 4: > + FIELD_MODIFY(CH_4, &val, tmp_val); > + break; > + case 5: > + FIELD_MODIFY(CH_5, &val, tmp_val); > + break; > + case 6: > + FIELD_MODIFY(CH_6, &val, tmp_val); > + break; > + case 7: > + FIELD_MODIFY(CH_7, &val, tmp_val); > + break; > + default: > + FIELD_MODIFY(CH_0, &val, tmp_val); > + break; > + } > + } > + > + ret = regmap_write(regmap, CMD_FORMAT(reg, WRITE_CMD), val); Looking at the above, this looks like a creative way of doing regmap_update_bits()? Am I missing something? > + > + return ret; return regmap_write() > +} > + > +static int mcp47feb02_write_to_eeprom(struct regmap *regmap, unsigned int reg, > + unsigned int val) > +{ > + int eewa_val, ret; > + > + /* > + * wait till the currently occurring EEPROM Write Cycle is completed. > + * Only serial commands to the volatile memory are allowed. > + */ > + ret = regmap_read_poll_timeout(regmap, > + CMD_FORMAT(MCP47FEB02_GAIN_STATUS_REG_ADDR, > READ_CMD), > + eewa_val, !(eewa_val & > MCP47FEB02_GAIN_STATUS_EEWA_MASK), > + MCP47FEB02_DELAY_1_MS, > MCP47FEB02_DELAY_1_MS * 5); > + if (ret) > + return ret; > + > + ret = regmap_write(regmap, CMD_FORMAT(reg, WRITE_CMD), val); > + > + return ret; > +} > + > +static ssize_t mcp47feb02_store_eeprom(struct device *dev, struct device_attribute > *attr, > + const char *buf, size_t len) > +{ > + struct mcp47feb02_data *data = iio_priv(dev_to_iio_dev(dev)); > + int ret, i, val, val1; > + bool state; > + > + ret = kstrtobool(buf, &state); > + if (ret < 0) > + return ret; > + > + if (!state) > + return 0; > + > + if (!data->have_eeprom) { > + dev_err(dev, "Device has no eeprom memory.\n"); > + return -EINVAL; > + } > + > + /* > + * Verify DAC Wiper and DAC Configuratioin are unlocked. If both are > disabled, > + * writing to EEPROM is available. > + */ > + ret = regmap_read(data->regmap, > CMD_FORMAT(MCP47FEB02_WIPERLOCK_STATUS_REG_ADDR, READ_CMD), > + &val); > + if (ret) > + return ret; > + > + if (val) { > + dev_err(dev, "DAC Wiper and DAC Configuration not are > unlocked.\n"); > + return -EINVAL; > + } > + > + for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) { > + ret = mcp47feb02_write_to_eeprom(data->regmap, > MCP47FEB02_NV_DAC0_REG_ADDR + i, > + data->chdata[i].dac_data); > + if (ret) > + return ret; > + } > + > + ret = regmap_read(data->regmap, CMD_FORMAT(MCP47FEB02_VREF_REG_ADDR, > READ_CMD), &val); > + if (ret) > + return ret; > + > + ret = mcp47feb02_write_to_eeprom(data->regmap, > MCP47FEB02_NV_VREF_REG_ADDR, val); > + if (ret) > + return ret; > + > + ret = regmap_read(data->regmap, CMD_FORMAT(MCP47FEB02_POWER_DOWN_REG_ADDR, > READ_CMD), &val); > + if (ret) > + return ret; > + > + ret = mcp47feb02_write_to_eeprom(data->regmap, > MCP47FEB02_NV_POWER_DOWN_REG_ADDR, val); > + if (ret) > + return ret; > + > + ret = regmap_read(data->regmap, > + CMD_FORMAT(MCP47FEB02_NV_GAIN_I2C_SLAVE_REG_ADDR, > READ_CMD), &val); > + if (ret) > + return ret; > + > + ret = regmap_read(data->regmap, > + CMD_FORMAT(MCP47FEB02_GAIN_STATUS_REG_ADDR, READ_CMD), > &val1); > + if (ret) > + return ret; > + > + ret = mcp47feb02_write_to_eeprom(data->regmap, > MCP47FEB02_NV_GAIN_I2C_SLAVE_REG_ADDR, > + (val1 & MCP47FEB02_VOLATILE_GAIN_MASK) | > + (val & > MCP47FEB02_NV_I2C_SLAVE_ADDR_MASK)); > + if (ret) > + return ret; > + > + return len; > +} > + > +static IIO_DEVICE_ATTR(store_eeprom, 0200, NULL, mcp47feb02_store_eeprom, 0); > +static struct attribute *mcp47feb02_attributes[] = { > + &iio_dev_attr_store_eeprom.dev_attr.attr, > + NULL, > +}; > + Not going to argue about the ABI for now but I don't think this is a standard one? So if acceptable you need an ABI doc. > +static int mcp47feb02_suspend(struct device *dev) > +{ > + struct iio_dev *indio_dev = dev_get_drvdata(dev); > + struct mcp47feb02_data *data = iio_priv(indio_dev); > + int ret, ch; > + > + for_each_set_bit(ch, &data->active_channels_mask, data->phys_channels) { > + data->chdata[ch].powerdown = true; > + ret = mcp47feb02_write_to_register(data->regmap, > MCP47FEB02_POWER_DOWN_REG_ADDR, > + ch, data- > >chdata[ch].powerdown_mode + 1); > + if (ret) > + return ret; So looking at this, can we actually powerdown channels individually? > + > + ret = regmap_write(data->regmap, CMD_FORMAT(ch, WRITE_CMD), > + data->chdata[ch].dac_data); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > +static int mcp47feb02_resume(struct device *dev) > +{ > + struct iio_dev *indio_dev = dev_get_drvdata(dev); > + struct mcp47feb02_data *data = iio_priv(indio_dev); > + int ch, ret; > + > + for_each_set_bit(ch, &data->active_channels_mask, data->phys_channels) { > + data->chdata[ch].powerdown = false; > + > + ret = regmap_write(data->regmap, CMD_FORMAT(ch, WRITE_CMD), > + data->chdata[ch].dac_data); > + if (ret) > + return ret; > + > + ret = mcp47feb02_write_to_register(data->regmap, > MCP47FEB02_VREF_REG_ADDR, > + ch, data->chdata[ch].ref_mode); > + if (ret) > + return ret; > + > + ret = mcp47feb02_write_to_register(data->regmap, > MCP47FEB02_GAIN_STATUS_REG_ADDR, > + ch, data- > >chdata[ch].use_2x_gain); > + if (ret) > + return ret; > + > + ret = mcp47feb02_write_to_register(data->regmap, > MCP47FEB02_POWER_DOWN_REG_ADDR, > + ch, > MCP47FEB02_NORMAL_OPERATION); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > +static DEFINE_SIMPLE_DEV_PM_OPS(mcp47feb02_pm_ops, mcp47feb02_suspend, > mcp47feb02_resume); > + > +static const struct attribute_group mcp47feb02_attribute_group = { > + .attrs = mcp47feb02_attributes, > +}; > + > +static const char * const mcp47feb02_powerdown_modes[] = { > + "1kohm_to_gnd", > + "100kohm_to_gnd", > + "open_circuit", > +}; > + > +static int mcp47feb02_get_powerdown_mode(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan) > +{ > + struct mcp47feb02_data *data = iio_priv(indio_dev); > + int bit_set; > + > + bit_set = test_bit(chan->address, &data->active_channels_mask); > + if (bit_set) not seeing the need for bit_set. > + return data->chdata[chan->address].powerdown_mode; > + > + return -EINVAL; > +} > + > +static int mcp47feb02_set_powerdown_mode(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan, > unsigned int mode) > +{ > + struct mcp47feb02_data *data = iio_priv(indio_dev); > + struct device *dev = &data->client->dev; > + int bit_set, ret; > + > + bit_set = test_bit(chan->address, &data->active_channels_mask); > + if (bit_set) { same > + ret = mcp47feb02_write_to_register(data->regmap, > MCP47FEB02_POWER_DOWN_REG_ADDR, > + chan->address, mode + 1); > + if (ret) > + return ret; > + > + data->chdata[chan->address].powerdown_mode = mode; > + > + return 0; > + } > + > + dev_err(dev, "Channel %ld not enabled\n", chan->address); > + return -EINVAL; > +} > + > +static ssize_t mcp47feb02_read_powerdown(struct iio_dev *indio_dev, uintptr_t > private, > + const struct iio_chan_spec *chan, char > *buf) > +{ > + struct mcp47feb02_data *data = iio_priv(indio_dev); > + int bit_set; > + > + bit_set = test_bit(chan->address, &data->active_channels_mask); > + if (bit_set) > + return sysfs_emit(buf, "%d\n", data->chdata[chan- > >address].powerdown); > + > + dev_err(&data->client->dev, "Channel %ld not enabled\n", chan->address); > + return -EINVAL; > +} > + > +static ssize_t mcp47feb02_write_powerdown(struct iio_dev *indio_dev, uintptr_t > private, > + const struct iio_chan_spec *chan, const > char *buf, > + size_t len) > +{ > + struct mcp47feb02_data *data = iio_priv(indio_dev); > + struct device *dev = &data->client->dev; > + int bit_set, ret; > + bool state; > + > + ret = kstrtobool(buf, &state); > + if (ret) > + return ret; > + > + bit_set = test_bit(chan->address, &data->active_channels_mask); > + if (bit_set) > + data->chdata[chan->address].powerdown = state; > + else > + dev_err(dev, "Channel %ld not enabled\n", chan->address); I might be missing something but I'm puzzled. So powering down is just setting a variable? I would expect some register read. > + > + return len; > +} > + > +static const struct iio_enum mcp47febxx_powerdown_mode_enum = { > + .items = mcp47feb02_powerdown_modes, > + .num_items = ARRAY_SIZE(mcp47feb02_powerdown_modes), > + .get = mcp47feb02_get_powerdown_mode, > + .set = mcp47feb02_set_powerdown_mode, > +}; > + > +static const struct iio_chan_spec_ext_info mcp47feb02_ext_info[] = { > + { > + .name = "powerdown", > + .read = mcp47feb02_read_powerdown, > + .write = mcp47feb02_write_powerdown, > + .shared = IIO_SEPARATE, > + }, > + IIO_ENUM("powerdown_mode", IIO_SEPARATE, &mcp47febxx_powerdown_mode_enum), > + IIO_ENUM_AVAILABLE("powerdown_mode", IIO_SHARED_BY_TYPE, > &mcp47febxx_powerdown_mode_enum), > + { }, > +}; > + > +static const struct iio_chan_spec mcp47febxx_ch_template = { > + .type = IIO_VOLTAGE, > + .output = 1, > + .indexed = 1, > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > + BIT(IIO_CHAN_INFO_SCALE), > + .info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE), > + .ext_info = mcp47feb02_ext_info, > +}; > + > +static void mcp47feb02_init_scale(const struct mcp47feb02_features *info, > + enum mcp47feb02_scale scale, int vref_mv, > + int scale_avail[]) > +{ > + int value_micro, value_int; > + s64 tmp; > + > + tmp = (s64)vref_mv * 1000000LL >> info->resolution; > + value_int = div_s64_rem(tmp, 1000000LL, &value_micro); > + scale_avail[scale * 2] = value_int; > + scale_avail[scale * 2 + 1] = value_micro; > +} > + > +static int mcp47feb02_init_scales_avail(const struct mcp47feb02_features *info, > + struct mcp47feb02_data *data, int vdd_mv, > int vref_mv, > + int vref1_mv) > +{ > + struct device *dev = &data->client->dev; > + int tmp_vref; > + > + if (data->use_vref && vref_mv <= 0) { > + dev_err(dev, "use_vref set but vref_mv invalid\n"); > + return -EINVAL; dev_err_probe()? > + } > + > + mcp47feb02_init_scale(info, MCP47FEB02_SCALE_VDD, vdd_mv, data->scale); > + > + if (data->use_vref) > + tmp_vref = vref_mv; > + else > + tmp_vref = MCP47FEB02_INTERNAL_BAND_GAP_MV; > + > + mcp47feb02_init_scale(info, MCP47FEB02_SCALE_GAIN_X1, tmp_vref, data- > >scale); > + mcp47feb02_init_scale(info, MCP47FEB02_SCALE_GAIN_X2, tmp_vref * 2, data- > >scale); > + > + if (data->phys_channels >= 4) { > + mcp47feb02_init_scale(info, MCP47FEB02_SCALE_VDD, vdd_mv, data- > >scale_1); > + > + if (data->use_vref1 && vref1_mv <= 0) { > + dev_err(dev, "use_vref1 set but vref1_mv invalid\n"); > + return -EINVAL; same ... > + > +static int mcp47feb02_set_scale(struct mcp47feb02_data *data, int channel, int > scale) > +{ > + int tmp_val, ret; > + > + mcp47feb02_ch_scale(data->phys_channels, data, channel, scale); > + > + if (scale == MCP47FEB02_SCALE_GAIN_X2) > + tmp_val = MCP47FEB02_GAIN_X2; > + else > + tmp_val = MCP47FEB02_GAIN_X1; > + > + ret = mcp47feb02_write_to_register(data->regmap, > MCP47FEB02_GAIN_STATUS_REG_ADDR, > + channel, tmp_val); > + if (ret) > + return ret; > + > + data->chdata[channel].use_2x_gain = tmp_val; > Also looks like it could use a lock. > + return 0; > +} > + > +static int mcp47feb02_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec > const *chan, > + int *val, int *val2, long mask) > +{ > + struct mcp47feb02_data *data = iio_priv(indio_dev); > + int ret; > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + ret = regmap_read(data->regmap, CMD_FORMAT(chan->address, > READ_CMD), val); > + if (ret) > + return ret; > + > + return IIO_VAL_INT; > + case IIO_CHAN_INFO_SCALE: > + mcp47feb02_get_scale(chan->address, data, val, val2); > + return IIO_VAL_INT_PLUS_MICRO; > + default: > + return -EINVAL; > + } > +} > + > +static int mcp47feb02_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec > const *chan, > + int val, int val2, long mask) > +{ > + struct mcp47feb02_data *data = iio_priv(indio_dev); > + int ret; > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + ret = regmap_write(data->regmap, CMD_FORMAT(chan->address, > WRITE_CMD), val); > + data->chdata[chan->address].dac_data = val; You should only save the value in case regmap_write() succeeds. And given that you're doing that you should use a mutx. > + return ret; > + case IIO_CHAN_INFO_SCALE: > + if (data->phys_channels >= 4 && (chan->address % 2)) > + ret = mcp47feb02_check_scale(data, val, val2, data- > >scale_1); > + else > + ret = mcp47feb02_check_scale(data, val, val2, data- > >scale); > + > + if (ret < 0) > + return ret; > + > + return mcp47feb02_set_scale(data, chan->address, ret); > + default: > + return -EINVAL; > + } > +} > + > +static const struct iio_info mcp47feb02_info = { > + .read_raw = mcp47feb02_read_raw, > + .write_raw = mcp47feb02_write_raw, > + .read_avail = &mcp47feb02_read_avail, > + .attrs = &mcp47feb02_attribute_group, > +}; > + > +static const struct iio_info mcp47fvb02_info = { > + .read_raw = mcp47feb02_read_raw, > + .write_raw = mcp47feb02_write_raw, > + .read_avail = &mcp47feb02_read_avail, > + .attrs = &mcp47feb02_attribute_group, > +}; > + > +static int mcp47feb02_parse_fw(struct iio_dev *indio_dev, const struct > mcp47feb02_features *info) > +{ > + struct iio_chan_spec chanspec = mcp47febxx_ch_template; > + struct mcp47feb02_data *data = iio_priv(indio_dev); > + struct device *dev = &data->client->dev; > + struct iio_chan_spec *channels; > + u32 reg, num_channels; > + int chan_idx = 0; > + > + num_channels = device_get_child_node_count(dev); > + if (num_channels > info->phys_channels) > + return dev_err_probe(dev, -EINVAL, "More channels than the chip > supports\n"); > + Are 0 channels acceptable? devm_kcalloc() wont't give you a NULL ptr and you might get subtle issues. > + channels = devm_kcalloc(dev, num_channels, sizeof(*channels), GFP_KERNEL); > + if (!channels) > + return -ENOMEM; > + > + device_for_each_child_node_scoped(dev, child) { > + fwnode_property_read_u32(child, "reg", ®); > + reg is a mandatory property so you need to check for errors. If the property is not given, fwnode_property_read_u32() won't touch in reg so you have a random number in the next condition. > + if (reg >= info->phys_channels) > + return dev_err_probe(dev, -EINVAL, > + "The index of the channels does not > match the chip\n"); > + > + set_bit(reg, &data->active_channels_mask); > + > + if (fwnode_property_present(child, "label")) > + fwnode_property_read_string(child, "label", > + (const char **)&data- > >labels[reg]); > + > + chanspec.address = reg; > + chanspec.channel = reg; > + channels[chan_idx] = chanspec; > + chan_idx++; > + } > + > + indio_dev->num_channels = num_channels; > + indio_dev->channels = channels; > + indio_dev->modes = INDIO_DIRECT_MODE; > + data->phys_channels = info->phys_channels; > + > + /* > + * check if vref-supply, vref1-supply, microchip,vref-buffered and > + * microchip,vref1-buffered are defined in the devicetree > + */ > + data->use_vref = device_property_present(dev, "vref-supply"); > + data->use_vref1 = device_property_present(dev, "vref1-supply"); > + data->vref_buffered = device_property_read_bool(dev, "microchip,vref- > buffered"); > + data->vref1_buffered = device_property_read_bool(dev, "microchip,vref1- > buffered"); > + > + return 0; > +} > + > +static int mcp47feb02_probe(struct i2c_client *client) > +{ > + int err, ret, vdd_mv, vref_mv, vref1_mv, i, tmp_vref, vref_ch, gain_ch; > + const struct i2c_device_id *id = i2c_client_get_device_id(client); > + const struct mcp47feb02_features *info; > + enum vref_mode ref_mode, ref_mode1; > + struct device *dev = &client->dev; > + struct mcp47feb02_data *data; > + struct iio_dev *indio_dev; > + > + indio_dev = devm_iio_device_alloc(dev, sizeof(*data)); > + if (!indio_dev) > + return -ENOMEM; > + > + data = iio_priv(indio_dev); > + i2c_set_clientdata(client, indio_dev); You can drop the above. See my devm_ comment below. > + data->client = client; > + info = i2c_get_match_data(client); for consistency, add some error handling. > + > + if (info->have_eeprom) { > + data->regmap = devm_regmap_init_i2c(client, > &mcp47feb02_regmap_config); > + data->have_eeprom = true; > + } else { > + data->regmap = devm_regmap_init_i2c(client, > &mcp47fvb02_regmap_config); > + data->have_eeprom = false; > + } > + > + if (IS_ERR(data->regmap)) > + dev_err_probe(dev, PTR_ERR(data->regmap), "Error initializing i2c > regmap\n"); > + > + err = mcp47feb02_parse_fw(indio_dev, info); > + if (err) > + return dev_err_probe(dev, err, "Error parsing devicetree data\n"); > + > + if (!info->have_ext_vref2 && data->use_vref1) > + return dev_err_probe(dev, -EINVAL, > + "Second External reference is unavailable on > %s\n", > + info->name); > + > + ret = regmap_read(data->regmap, CMD_FORMAT(MCP47FEB02_VREF_REG_ADDR, > READ_CMD), &vref_ch); > + if (ret) > + return ret; > + > + ret = regmap_read(data->regmap, > CMD_FORMAT(MCP47FEB02_GAIN_STATUS_REG_ADDR, READ_CMD), > + &gain_ch); > + if (ret) > + return ret; > + > + gain_ch = gain_ch >> 8; > + > + /* > + * Values stored in the nonvolatile memory will be transferred to the > volatile registers > + * at startup. For safety reasons, the user receives a warning if startup > values > + * do not match the ones from current devicetree configuration. > + * Nonvolatile memory can be written at any time > + */ > + for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) { > + /* VDD can be set as Vref only with Gain x1 */ > + if ((vref_ch & 0x03) == MCP47FEB02_VREF_VDD && > + (gain_ch & 0x01) == MCP47FEB02_GAIN_X2) { some defines with be nice for those magic bits. > + dev_info(dev, "vdd can be used only with gain x1\n"); dev_dbg() > + ret = mcp47feb02_write_to_register(data->regmap, > + > MCP47FEB02_GAIN_STATUS_REG_ADDR, > + i, MCP47FEB02_GAIN_X1); > + if (ret) > + return ret; > + > + data->chdata[i].use_2x_gain = MCP47FEB02_GAIN_X1; > + } > + > + if (data->phys_channels >= 4 && (i % 2)) { > + if ((vref_ch & 0x03) == MCP47FEB02_EXTERNAL_VREF_BUFFERED > && > + data->use_vref1 && !data->vref1_buffered) > + dev_info(dev, "vref1 is unbuffered\n"); > + else if ((vref_ch & 0x03) == > MCP47FEB02_EXTERNAL_VREF_UNBUFFERED && > + data->use_vref1 && data->vref1_buffered) > + dev_info(dev, "vref1 is buffered\n"); > + } else { > + if ((vref_ch & 0x03) == MCP47FEB02_EXTERNAL_VREF_BUFFERED > && > + data->use_vref && !data->vref_buffered) > + dev_info(dev, "vref is unbuffered\n"); > + else if ((vref_ch & 0x03) == > MCP47FEB02_EXTERNAL_VREF_UNBUFFERED && > + data->use_vref && data->vref_buffered) > + dev_info(dev, "vref is buffered\n"); > + } > + > + vref_ch = vref_ch >> 2; > + gain_ch = gain_ch >> 1; > + } > + > + if (data->use_vref) > + ref_mode = data->vref_buffered ? > + MCP47FEB02_EXTERNAL_VREF_BUFFERED : > MCP47FEB02_EXTERNAL_VREF_UNBUFFERED; > + else > + ref_mode = MCP47FEB02_INTERNAL_BAND_GAP; > + > + if (data->use_vref1) > + ref_mode1 = data->vref1_buffered ? > + MCP47FEB02_EXTERNAL_VREF_BUFFERED : > MCP47FEB02_EXTERNAL_VREF_UNBUFFERED; > + > + else > + ref_mode1 = MCP47FEB02_INTERNAL_BAND_GAP; > + > + for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) { > + if (data->phys_channels >= 4 && (i % 2)) > + tmp_vref = ref_mode1; > + else > + tmp_vref = ref_mode; > + > + ret = mcp47feb02_write_to_register(data->regmap, > MCP47FEB02_VREF_REG_ADDR, > + i, tmp_vref); > + if (ret) > + return ret; > + > + data->chdata[i].ref_mode = tmp_vref; > + } Maybe have the above in a setup function? > + > + indio_dev->name = id->name; > + if (info->have_eeprom) > + indio_dev->info = &mcp47feb02_info; > + else > + indio_dev->info = &mcp47fvb02_info; > + > + ret = devm_mutex_init(dev, &data->lock); > + if (ret < 0) > + return ret; > + > + ret = devm_regulator_get_enable_read_voltage(dev, "vdd"); > + if (ret < 0) > + return ret; > + > + vdd_mv = ret / 1000; > + > + if (data->use_vref) { > + ret = devm_regulator_get_enable_read_voltage(dev, "vref"); > + if (ret < 0) > + return ret; > + > + vref_mv = ret / 1000; > + } > + > + if (data->use_vref1) { > + ret = devm_regulator_get_enable_read_voltage(dev, "vref1"); > + if (ret < 0) > + return ret; > + > + vref1_mv = ret / 1000; > + } > + > + for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) { > + ret = mcp47feb02_init_scales_avail(info, data, vdd_mv, vref_mv, > vref1_mv); > + if (ret) > + dev_err_probe(dev, ret, "failed to init scales for ch i > %d\n", i); > + } I guess the above loop could also be in a setup function. > + > + err = iio_device_register(indio_dev); > + devm_iio_device_register(). With that you can drop mcp47feb02_remove() - Nuno Sá ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] iio: dac: adding support for Microchip MCP47FEB02 2025-09-22 20:10 ` Nuno Sá @ 2025-09-22 22:15 ` David Lechner 2025-09-23 8:21 ` Nuno Sá 0 siblings, 1 reply; 14+ messages in thread From: David Lechner @ 2025-09-22 22:15 UTC (permalink / raw) To: Nuno Sá, Ariana Lazar, Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-iio, devicetree, linux-kernel On 9/22/25 3:10 PM, Nuno Sá wrote: > Hi Ariana, > > Thanks for your patches. Some initial comments from me... > > On Mon, 2025-09-22 at 14:30 +0300, Ariana Lazar wrote: ... >> +static IIO_DEVICE_ATTR(store_eeprom, 0200, NULL, mcp47feb02_store_eeprom, 0); >> +static struct attribute *mcp47feb02_attributes[] = { >> + &iio_dev_attr_store_eeprom.dev_attr.attr, >> + NULL, >> +}; >> + > > Not going to argue about the ABI for now but I don't think this is a standard one? So > if acceptable you need an ABI doc. > Here's a random idea. (I would wait for Jonathan to weigh in first before assuming it is an acceptable idea though :-p) The config registers are pretty much going to be a one-time deal. So those could be written to only if they need it during probe. For the voltage output registers, we could add extra out_voltageY channels that are the power-on output state channels. So writing to out_voltageY_raw wouldn't change any real output but would just be written to EEPROM. This way these voltages could be controlled independently from the real outputs and it uses existing ABI. In any case, it would be interesting to hear more about how this chips are actually used to better understand this EEPROM feature. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] iio: dac: adding support for Microchip MCP47FEB02 2025-09-22 22:15 ` David Lechner @ 2025-09-23 8:21 ` Nuno Sá 2025-09-27 17:13 ` Jonathan Cameron 0 siblings, 1 reply; 14+ messages in thread From: Nuno Sá @ 2025-09-23 8:21 UTC (permalink / raw) To: David Lechner, Ariana Lazar, Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-iio, devicetree, linux-kernel On Mon, 2025-09-22 at 17:15 -0500, David Lechner wrote: > On 9/22/25 3:10 PM, Nuno Sá wrote: > > Hi Ariana, > > > > Thanks for your patches. Some initial comments from me... > > > > On Mon, 2025-09-22 at 14:30 +0300, Ariana Lazar wrote: > > ... > > > > +static IIO_DEVICE_ATTR(store_eeprom, 0200, NULL, mcp47feb02_store_eeprom, > > > 0); > > > +static struct attribute *mcp47feb02_attributes[] = { > > > + &iio_dev_attr_store_eeprom.dev_attr.attr, > > > + NULL, > > > +}; > > > + > > > > Not going to argue about the ABI for now but I don't think this is a > > standard one? So > > if acceptable you need an ABI doc. > > > Here's a random idea. (I would wait for Jonathan to weigh in first before > assuming it is an acceptable idea though :-p) > > The config registers are pretty much going to be a one-time deal. So those > could be written to only if they need it during probe. > > For the voltage output registers, we could add extra out_voltageY channels > that are the power-on output state channels. So writing to out_voltageY_raw > wouldn't change any real output but would just be written to EEPROM. This > way these voltages could be controlled independently from the real outputs > and it uses existing ABI. > > In any case, it would be interesting to hear more about how this chips are > actually used to better understand this EEPROM feature. I didn't really looked at the datasheet so this can be totally wrong. But we have some LTC parts (mainly hwmon stuff) that are also packed with an EEPRON. AFAIU, the usecase in there is to have some defaults you can program in the chips (and there's a feature we can enable so the chip can save things into the eeprom automatically). Now, in those drivers we don't really support doing anything with the eeprom at runtime so I'm curious to see how this unfolds :) - Nuno Sá ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] iio: dac: adding support for Microchip MCP47FEB02 2025-09-23 8:21 ` Nuno Sá @ 2025-09-27 17:13 ` Jonathan Cameron 2025-09-29 5:44 ` Nuno Sá 0 siblings, 1 reply; 14+ messages in thread From: Jonathan Cameron @ 2025-09-27 17:13 UTC (permalink / raw) To: Nuno Sá Cc: David Lechner, Ariana Lazar, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio, devicetree, linux-kernel On Tue, 23 Sep 2025 09:21:30 +0100 Nuno Sá <noname.nuno@gmail.com> wrote: > On Mon, 2025-09-22 at 17:15 -0500, David Lechner wrote: > > On 9/22/25 3:10 PM, Nuno Sá wrote: > > > Hi Ariana, > > > > > > Thanks for your patches. Some initial comments from me... > > > > > > On Mon, 2025-09-22 at 14:30 +0300, Ariana Lazar wrote: > > > > ... > > > > > > +static IIO_DEVICE_ATTR(store_eeprom, 0200, NULL, mcp47feb02_store_eeprom, > > > > 0); > > > > +static struct attribute *mcp47feb02_attributes[] = { > > > > + &iio_dev_attr_store_eeprom.dev_attr.attr, > > > > + NULL, > > > > +}; > > > > + > > > > > > Not going to argue about the ABI for now but I don't think this is a > > > standard one? So > > > if acceptable you need an ABI doc. store_eeprom is existing ABI and documented in sysfs-bus-iio (2 drivers implement it from a quick grep) > > > > > Here's a random idea. (I would wait for Jonathan to weigh in first before > > assuming it is an acceptable idea though :-p) > > > > The config registers are pretty much going to be a one-time deal. So those > > could be written to only if they need it during probe. > > > > For the voltage output registers, we could add extra out_voltageY channels > > that are the power-on output state channels. So writing to out_voltageY_raw > > wouldn't change any real output but would just be written to EEPROM. This > > way these voltages could be controlled independently from the real outputs > > and it uses existing ABI. In some devices I've come across, the eeprom write is a 'store all current settings' rather than individual register writes. For that a set of extra channels doesn't work. Also eeproms have very limited write cycles so you really don't want to make this too easy to do and we want to shout it's an eeprom. > > > > In any case, it would be interesting to hear more about how this chips are > > actually used to better understand this EEPROM feature. > > I didn't really looked at the datasheet so this can be totally wrong. But we > have some LTC parts (mainly hwmon stuff) that are also packed with an EEPRON. > AFAIU, the usecase in there is to have some defaults you can program in the > chips (and there's a feature we can enable so the chip can save things into the > eeprom automatically). Now, in those drivers we don't really support doing > anything with the eeprom at runtime so I'm curious to see how this unfolds :) Usecase for DACs is that on power on (usually at board power up not driver load) they will start outputting the saved values. That might well be part of something fairly critical such as fan control or trip points so you don't want to wait for the driver to load. The driver on an eeprom equiped part should not configure anything on probe but rather just report back what was already there. Userspace can then modify those values and 'commit' them via store_eeprom to apply on next power cycle as well as now (in some cases only on next power cycle). Jonathan > > - Nuno Sá ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] iio: dac: adding support for Microchip MCP47FEB02 2025-09-27 17:13 ` Jonathan Cameron @ 2025-09-29 5:44 ` Nuno Sá 0 siblings, 0 replies; 14+ messages in thread From: Nuno Sá @ 2025-09-29 5:44 UTC (permalink / raw) To: Jonathan Cameron Cc: David Lechner, Ariana Lazar, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio, devicetree, linux-kernel On Sat, 2025-09-27 at 18:13 +0100, Jonathan Cameron wrote: > On Tue, 23 Sep 2025 09:21:30 +0100 > Nuno Sá <noname.nuno@gmail.com> wrote: > > > On Mon, 2025-09-22 at 17:15 -0500, David Lechner wrote: > > > On 9/22/25 3:10 PM, Nuno Sá wrote: > > > > Hi Ariana, > > > > > > > > Thanks for your patches. Some initial comments from me... > > > > > > > > On Mon, 2025-09-22 at 14:30 +0300, Ariana Lazar wrote: > > > > > > ... > > > > > > > > +static IIO_DEVICE_ATTR(store_eeprom, 0200, NULL, mcp47feb02_store_eeprom, > > > > > 0); > > > > > +static struct attribute *mcp47feb02_attributes[] = { > > > > > + &iio_dev_attr_store_eeprom.dev_attr.attr, > > > > > + NULL, > > > > > +}; > > > > > + > > > > > > > > Not going to argue about the ABI for now but I don't think this is a > > > > standard one? So > > > > if acceptable you need an ABI doc. > > store_eeprom is existing ABI and documented in sysfs-bus-iio (2 drivers implement > it from > a quick grep) > Ack. Next time I should bother in grepping the docs :) > > > > > > > > Here's a random idea. (I would wait for Jonathan to weigh in first before > > > assuming it is an acceptable idea though :-p) > > > > > > The config registers are pretty much going to be a one-time deal. So those > > > could be written to only if they need it during probe. > > > > > > For the voltage output registers, we could add extra out_voltageY channels > > > that are the power-on output state channels. So writing to out_voltageY_raw > > > wouldn't change any real output but would just be written to EEPROM. This > > > way these voltages could be controlled independently from the real outputs > > > and it uses existing ABI. > > In some devices I've come across, the eeprom write is a 'store all current > settings' > rather than individual register writes. For that a set of extra channels doesn't > work. > > Also eeproms have very limited write cycles so you really don't want to make this > too easy to do and we want to shout it's an eeprom. > > > > > > > > In any case, it would be interesting to hear more about how this chips are > > > actually used to better understand this EEPROM feature. > > > > I didn't really looked at the datasheet so this can be totally wrong. But we > > have some LTC parts (mainly hwmon stuff) that are also packed with an EEPRON. > > AFAIU, the usecase in there is to have some defaults you can program in the > > chips (and there's a feature we can enable so the chip can save things into the > > eeprom automatically). Now, in those drivers we don't really support doing > > anything with the eeprom at runtime so I'm curious to see how this unfolds :) > > Usecase for DACs is that on power on (usually at board power up not driver load) > they will start outputting the saved values. That might well be part of something > fairly critical such as fan control or trip points so you don't want to wait for > the driver to load. The driver on an eeprom equiped part should not configure > anything on probe but rather just report back what was already there. > > Userspace can then modify those values and 'commit' them via store_eeprom > to apply on next power cycle as well as now (in some cases only on next power > cycle). Makes sense and it is a similar case of we have in hwmon given those LTC chips are often controlling power of some fundamental blocks of the system. - Nuno Sá > > Jonathan > > > > > > - Nuno Sá > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] iio: dac: adding support for Microchip MCP47FEB02 2025-09-22 11:30 ` [PATCH 2/2] " Ariana Lazar 2025-09-22 20:10 ` Nuno Sá @ 2025-09-26 15:38 ` Dan Carpenter 2025-09-27 17:53 ` Jonathan Cameron 2 siblings, 0 replies; 14+ messages in thread From: Dan Carpenter @ 2025-09-26 15:38 UTC (permalink / raw) To: oe-kbuild, Ariana Lazar, Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: lkp, oe-kbuild-all, linux-iio, devicetree, linux-kernel, Ariana Lazar Hi Ariana, kernel test robot noticed the following build warnings: url: https://github.com/intel-lab-lkp/linux/commits/Ariana-Lazar/dt-bindings-iio-dac-adding-support-for-Microchip-MCP47FEB02/20250922-193559 base: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494 patch link: https://lore.kernel.org/r/20250922-mcp47feb02-v1-2-06cb4acaa347%40microchip.com patch subject: [PATCH 2/2] iio: dac: adding support for Microchip MCP47FEB02 config: x86_64-randconfig-r073-20250926 (https://download.01.org/0day-ci/archive/20250926/202509262228.MYSY2WkV-lkp@intel.com/config) compiler: gcc-14 (Debian 14.2.0-19) 14.2.0 If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org> | Closes: https://lore.kernel.org/r/202509262228.MYSY2WkV-lkp@intel.com/ smatch warnings: drivers/iio/dac/mcp47feb02.c:1258 mcp47feb02_probe() error: uninitialized symbol 'vref_mv'. drivers/iio/dac/mcp47feb02.c:1258 mcp47feb02_probe() error: uninitialized symbol 'vref1_mv'. vim +/vref_mv +1258 drivers/iio/dac/mcp47feb02.c 2e305393740054 Ariana Lazar 2025-09-22 1108 static int mcp47feb02_probe(struct i2c_client *client) 2e305393740054 Ariana Lazar 2025-09-22 1109 { 2e305393740054 Ariana Lazar 2025-09-22 1110 int err, ret, vdd_mv, vref_mv, vref1_mv, i, tmp_vref, vref_ch, gain_ch; 2e305393740054 Ariana Lazar 2025-09-22 1111 const struct i2c_device_id *id = i2c_client_get_device_id(client); 2e305393740054 Ariana Lazar 2025-09-22 1112 const struct mcp47feb02_features *info; 2e305393740054 Ariana Lazar 2025-09-22 1113 enum vref_mode ref_mode, ref_mode1; 2e305393740054 Ariana Lazar 2025-09-22 1114 struct device *dev = &client->dev; 2e305393740054 Ariana Lazar 2025-09-22 1115 struct mcp47feb02_data *data; 2e305393740054 Ariana Lazar 2025-09-22 1116 struct iio_dev *indio_dev; 2e305393740054 Ariana Lazar 2025-09-22 1117 2e305393740054 Ariana Lazar 2025-09-22 1118 indio_dev = devm_iio_device_alloc(dev, sizeof(*data)); 2e305393740054 Ariana Lazar 2025-09-22 1119 if (!indio_dev) 2e305393740054 Ariana Lazar 2025-09-22 1120 return -ENOMEM; 2e305393740054 Ariana Lazar 2025-09-22 1121 2e305393740054 Ariana Lazar 2025-09-22 1122 data = iio_priv(indio_dev); 2e305393740054 Ariana Lazar 2025-09-22 1123 i2c_set_clientdata(client, indio_dev); 2e305393740054 Ariana Lazar 2025-09-22 1124 data->client = client; 2e305393740054 Ariana Lazar 2025-09-22 1125 info = i2c_get_match_data(client); 2e305393740054 Ariana Lazar 2025-09-22 1126 2e305393740054 Ariana Lazar 2025-09-22 1127 if (info->have_eeprom) { 2e305393740054 Ariana Lazar 2025-09-22 1128 data->regmap = devm_regmap_init_i2c(client, &mcp47feb02_regmap_config); 2e305393740054 Ariana Lazar 2025-09-22 1129 data->have_eeprom = true; 2e305393740054 Ariana Lazar 2025-09-22 1130 } else { 2e305393740054 Ariana Lazar 2025-09-22 1131 data->regmap = devm_regmap_init_i2c(client, &mcp47fvb02_regmap_config); 2e305393740054 Ariana Lazar 2025-09-22 1132 data->have_eeprom = false; 2e305393740054 Ariana Lazar 2025-09-22 1133 } 2e305393740054 Ariana Lazar 2025-09-22 1134 2e305393740054 Ariana Lazar 2025-09-22 1135 if (IS_ERR(data->regmap)) 2e305393740054 Ariana Lazar 2025-09-22 1136 dev_err_probe(dev, PTR_ERR(data->regmap), "Error initializing i2c regmap\n"); 2e305393740054 Ariana Lazar 2025-09-22 1137 2e305393740054 Ariana Lazar 2025-09-22 1138 err = mcp47feb02_parse_fw(indio_dev, info); 2e305393740054 Ariana Lazar 2025-09-22 1139 if (err) 2e305393740054 Ariana Lazar 2025-09-22 1140 return dev_err_probe(dev, err, "Error parsing devicetree data\n"); 2e305393740054 Ariana Lazar 2025-09-22 1141 2e305393740054 Ariana Lazar 2025-09-22 1142 if (!info->have_ext_vref2 && data->use_vref1) 2e305393740054 Ariana Lazar 2025-09-22 1143 return dev_err_probe(dev, -EINVAL, 2e305393740054 Ariana Lazar 2025-09-22 1144 "Second External reference is unavailable on %s\n", 2e305393740054 Ariana Lazar 2025-09-22 1145 info->name); 2e305393740054 Ariana Lazar 2025-09-22 1146 2e305393740054 Ariana Lazar 2025-09-22 1147 ret = regmap_read(data->regmap, CMD_FORMAT(MCP47FEB02_VREF_REG_ADDR, READ_CMD), &vref_ch); 2e305393740054 Ariana Lazar 2025-09-22 1148 if (ret) 2e305393740054 Ariana Lazar 2025-09-22 1149 return ret; 2e305393740054 Ariana Lazar 2025-09-22 1150 2e305393740054 Ariana Lazar 2025-09-22 1151 ret = regmap_read(data->regmap, CMD_FORMAT(MCP47FEB02_GAIN_STATUS_REG_ADDR, READ_CMD), 2e305393740054 Ariana Lazar 2025-09-22 1152 &gain_ch); 2e305393740054 Ariana Lazar 2025-09-22 1153 if (ret) 2e305393740054 Ariana Lazar 2025-09-22 1154 return ret; 2e305393740054 Ariana Lazar 2025-09-22 1155 2e305393740054 Ariana Lazar 2025-09-22 1156 gain_ch = gain_ch >> 8; 2e305393740054 Ariana Lazar 2025-09-22 1157 2e305393740054 Ariana Lazar 2025-09-22 1158 /* 2e305393740054 Ariana Lazar 2025-09-22 1159 * Values stored in the nonvolatile memory will be transferred to the volatile registers 2e305393740054 Ariana Lazar 2025-09-22 1160 * at startup. For safety reasons, the user receives a warning if startup values 2e305393740054 Ariana Lazar 2025-09-22 1161 * do not match the ones from current devicetree configuration. 2e305393740054 Ariana Lazar 2025-09-22 1162 * Nonvolatile memory can be written at any time 2e305393740054 Ariana Lazar 2025-09-22 1163 */ 2e305393740054 Ariana Lazar 2025-09-22 1164 for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) { 2e305393740054 Ariana Lazar 2025-09-22 1165 /* VDD can be set as Vref only with Gain x1 */ 2e305393740054 Ariana Lazar 2025-09-22 1166 if ((vref_ch & 0x03) == MCP47FEB02_VREF_VDD && 2e305393740054 Ariana Lazar 2025-09-22 1167 (gain_ch & 0x01) == MCP47FEB02_GAIN_X2) { 2e305393740054 Ariana Lazar 2025-09-22 1168 dev_info(dev, "vdd can be used only with gain x1\n"); 2e305393740054 Ariana Lazar 2025-09-22 1169 ret = mcp47feb02_write_to_register(data->regmap, 2e305393740054 Ariana Lazar 2025-09-22 1170 MCP47FEB02_GAIN_STATUS_REG_ADDR, 2e305393740054 Ariana Lazar 2025-09-22 1171 i, MCP47FEB02_GAIN_X1); 2e305393740054 Ariana Lazar 2025-09-22 1172 if (ret) 2e305393740054 Ariana Lazar 2025-09-22 1173 return ret; 2e305393740054 Ariana Lazar 2025-09-22 1174 2e305393740054 Ariana Lazar 2025-09-22 1175 data->chdata[i].use_2x_gain = MCP47FEB02_GAIN_X1; 2e305393740054 Ariana Lazar 2025-09-22 1176 } 2e305393740054 Ariana Lazar 2025-09-22 1177 2e305393740054 Ariana Lazar 2025-09-22 1178 if (data->phys_channels >= 4 && (i % 2)) { 2e305393740054 Ariana Lazar 2025-09-22 1179 if ((vref_ch & 0x03) == MCP47FEB02_EXTERNAL_VREF_BUFFERED && 2e305393740054 Ariana Lazar 2025-09-22 1180 data->use_vref1 && !data->vref1_buffered) 2e305393740054 Ariana Lazar 2025-09-22 1181 dev_info(dev, "vref1 is unbuffered\n"); 2e305393740054 Ariana Lazar 2025-09-22 1182 else if ((vref_ch & 0x03) == MCP47FEB02_EXTERNAL_VREF_UNBUFFERED && 2e305393740054 Ariana Lazar 2025-09-22 1183 data->use_vref1 && data->vref1_buffered) 2e305393740054 Ariana Lazar 2025-09-22 1184 dev_info(dev, "vref1 is buffered\n"); 2e305393740054 Ariana Lazar 2025-09-22 1185 } else { 2e305393740054 Ariana Lazar 2025-09-22 1186 if ((vref_ch & 0x03) == MCP47FEB02_EXTERNAL_VREF_BUFFERED && 2e305393740054 Ariana Lazar 2025-09-22 1187 data->use_vref && !data->vref_buffered) 2e305393740054 Ariana Lazar 2025-09-22 1188 dev_info(dev, "vref is unbuffered\n"); 2e305393740054 Ariana Lazar 2025-09-22 1189 else if ((vref_ch & 0x03) == MCP47FEB02_EXTERNAL_VREF_UNBUFFERED && 2e305393740054 Ariana Lazar 2025-09-22 1190 data->use_vref && data->vref_buffered) 2e305393740054 Ariana Lazar 2025-09-22 1191 dev_info(dev, "vref is buffered\n"); 2e305393740054 Ariana Lazar 2025-09-22 1192 } 2e305393740054 Ariana Lazar 2025-09-22 1193 2e305393740054 Ariana Lazar 2025-09-22 1194 vref_ch = vref_ch >> 2; 2e305393740054 Ariana Lazar 2025-09-22 1195 gain_ch = gain_ch >> 1; 2e305393740054 Ariana Lazar 2025-09-22 1196 } 2e305393740054 Ariana Lazar 2025-09-22 1197 2e305393740054 Ariana Lazar 2025-09-22 1198 if (data->use_vref) 2e305393740054 Ariana Lazar 2025-09-22 1199 ref_mode = data->vref_buffered ? 2e305393740054 Ariana Lazar 2025-09-22 1200 MCP47FEB02_EXTERNAL_VREF_BUFFERED : MCP47FEB02_EXTERNAL_VREF_UNBUFFERED; 2e305393740054 Ariana Lazar 2025-09-22 1201 else 2e305393740054 Ariana Lazar 2025-09-22 1202 ref_mode = MCP47FEB02_INTERNAL_BAND_GAP; 2e305393740054 Ariana Lazar 2025-09-22 1203 2e305393740054 Ariana Lazar 2025-09-22 1204 if (data->use_vref1) 2e305393740054 Ariana Lazar 2025-09-22 1205 ref_mode1 = data->vref1_buffered ? 2e305393740054 Ariana Lazar 2025-09-22 1206 MCP47FEB02_EXTERNAL_VREF_BUFFERED : MCP47FEB02_EXTERNAL_VREF_UNBUFFERED; 2e305393740054 Ariana Lazar 2025-09-22 1207 2e305393740054 Ariana Lazar 2025-09-22 1208 else 2e305393740054 Ariana Lazar 2025-09-22 1209 ref_mode1 = MCP47FEB02_INTERNAL_BAND_GAP; 2e305393740054 Ariana Lazar 2025-09-22 1210 2e305393740054 Ariana Lazar 2025-09-22 1211 for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) { 2e305393740054 Ariana Lazar 2025-09-22 1212 if (data->phys_channels >= 4 && (i % 2)) 2e305393740054 Ariana Lazar 2025-09-22 1213 tmp_vref = ref_mode1; 2e305393740054 Ariana Lazar 2025-09-22 1214 else 2e305393740054 Ariana Lazar 2025-09-22 1215 tmp_vref = ref_mode; 2e305393740054 Ariana Lazar 2025-09-22 1216 2e305393740054 Ariana Lazar 2025-09-22 1217 ret = mcp47feb02_write_to_register(data->regmap, MCP47FEB02_VREF_REG_ADDR, 2e305393740054 Ariana Lazar 2025-09-22 1218 i, tmp_vref); 2e305393740054 Ariana Lazar 2025-09-22 1219 if (ret) 2e305393740054 Ariana Lazar 2025-09-22 1220 return ret; 2e305393740054 Ariana Lazar 2025-09-22 1221 2e305393740054 Ariana Lazar 2025-09-22 1222 data->chdata[i].ref_mode = tmp_vref; 2e305393740054 Ariana Lazar 2025-09-22 1223 } 2e305393740054 Ariana Lazar 2025-09-22 1224 2e305393740054 Ariana Lazar 2025-09-22 1225 indio_dev->name = id->name; 2e305393740054 Ariana Lazar 2025-09-22 1226 if (info->have_eeprom) 2e305393740054 Ariana Lazar 2025-09-22 1227 indio_dev->info = &mcp47feb02_info; 2e305393740054 Ariana Lazar 2025-09-22 1228 else 2e305393740054 Ariana Lazar 2025-09-22 1229 indio_dev->info = &mcp47fvb02_info; 2e305393740054 Ariana Lazar 2025-09-22 1230 2e305393740054 Ariana Lazar 2025-09-22 1231 ret = devm_mutex_init(dev, &data->lock); 2e305393740054 Ariana Lazar 2025-09-22 1232 if (ret < 0) 2e305393740054 Ariana Lazar 2025-09-22 1233 return ret; 2e305393740054 Ariana Lazar 2025-09-22 1234 2e305393740054 Ariana Lazar 2025-09-22 1235 ret = devm_regulator_get_enable_read_voltage(dev, "vdd"); 2e305393740054 Ariana Lazar 2025-09-22 1236 if (ret < 0) 2e305393740054 Ariana Lazar 2025-09-22 1237 return ret; 2e305393740054 Ariana Lazar 2025-09-22 1238 2e305393740054 Ariana Lazar 2025-09-22 1239 vdd_mv = ret / 1000; 2e305393740054 Ariana Lazar 2025-09-22 1240 2e305393740054 Ariana Lazar 2025-09-22 1241 if (data->use_vref) { 2e305393740054 Ariana Lazar 2025-09-22 1242 ret = devm_regulator_get_enable_read_voltage(dev, "vref"); 2e305393740054 Ariana Lazar 2025-09-22 1243 if (ret < 0) 2e305393740054 Ariana Lazar 2025-09-22 1244 return ret; 2e305393740054 Ariana Lazar 2025-09-22 1245 2e305393740054 Ariana Lazar 2025-09-22 1246 vref_mv = ret / 1000; 2e305393740054 Ariana Lazar 2025-09-22 1247 } uninitialized on else path. 2e305393740054 Ariana Lazar 2025-09-22 1248 2e305393740054 Ariana Lazar 2025-09-22 1249 if (data->use_vref1) { 2e305393740054 Ariana Lazar 2025-09-22 1250 ret = devm_regulator_get_enable_read_voltage(dev, "vref1"); 2e305393740054 Ariana Lazar 2025-09-22 1251 if (ret < 0) 2e305393740054 Ariana Lazar 2025-09-22 1252 return ret; 2e305393740054 Ariana Lazar 2025-09-22 1253 2e305393740054 Ariana Lazar 2025-09-22 1254 vref1_mv = ret / 1000; 2e305393740054 Ariana Lazar 2025-09-22 1255 } vref1_mv not initialized on else path. 2e305393740054 Ariana Lazar 2025-09-22 1256 2e305393740054 Ariana Lazar 2025-09-22 1257 for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) { 2e305393740054 Ariana Lazar 2025-09-22 @1258 ret = mcp47feb02_init_scales_avail(info, data, vdd_mv, vref_mv, vref1_mv); It doesn't matter that mcp47feb02_init_scales_avail() checks data->use_vref and data->use_vref1. It's considered a bug anyway because the function is not marked as __always_inline. UBSan will complain at runtime as well. 2e305393740054 Ariana Lazar 2025-09-22 1259 if (ret) 2e305393740054 Ariana Lazar 2025-09-22 1260 dev_err_probe(dev, ret, "failed to init scales for ch i %d\n", i); 2e305393740054 Ariana Lazar 2025-09-22 1261 } 2e305393740054 Ariana Lazar 2025-09-22 1262 2e305393740054 Ariana Lazar 2025-09-22 1263 err = iio_device_register(indio_dev); 2e305393740054 Ariana Lazar 2025-09-22 1264 2e305393740054 Ariana Lazar 2025-09-22 1265 return err; 2e305393740054 Ariana Lazar 2025-09-22 1266 } -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] iio: dac: adding support for Microchip MCP47FEB02 2025-09-22 11:30 ` [PATCH 2/2] " Ariana Lazar 2025-09-22 20:10 ` Nuno Sá 2025-09-26 15:38 ` Dan Carpenter @ 2025-09-27 17:53 ` Jonathan Cameron 2 siblings, 0 replies; 14+ messages in thread From: Jonathan Cameron @ 2025-09-27 17:53 UTC (permalink / raw) To: Ariana Lazar Cc: David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio, devicetree, linux-kernel On Mon, 22 Sep 2025 14:30:54 +0300 Ariana Lazar <ariana.lazar@microchip.com> wrote: > This is the iio driver for Microchip MCP47F(E/V)B(0/1/2)1, MCP47F(E/V)B(0/1/2)2, > MCP47F(E/V)B(0/1/2)4 and MCP47F(E/V)B(0/1/2)8 series of buffered voltage output > Digital-to-Analog Converters with nonvolatile or volatile memory and an I2C > Interface. > > The families support up to 8 output channels. > > The devices can be 8-bit, 10-bit and 12-bit. > > Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com> Main feedback here is probably the stuff around the unusual regmap handling. If you need to set extra bits for reads over writes, then regmap provides the means to do that. If it is more custom than I'm understanding then use a custom regmap so that you can pass the actual register addresses rather than some composite of several things as the address. > --- ... > diff --git a/drivers/iio/dac/mcp47feb02.c b/drivers/iio/dac/mcp47feb02.c > new file mode 100644 > index 0000000000000000000000000000000000000000..c9c2ded78d9c6013e5618e6342ebc8f50e79a31e > --- /dev/null > +++ b/drivers/iio/dac/mcp47feb02.c > @@ -0,0 +1,1347 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * IIO driver for MCP47FEB02 Multi-Channel DAC with I2C interface > + * > + * Copyright (C) 2025 Microchip Technology Inc. and its subsidiaries > + * > + * Author: Ariana Lazar <ariana.lazar@microchip.com> > + * > + * Datasheet for MCP47FEBXX can be found here: > + * https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005375A.pdf > + * > + * Datasheet for MCP47FVBXX can be found here: > + * https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005405A.pdf > + * > + * Datasheet for MCP47FXBX4/8 can be found here: > + * https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/MCP47FXBX48-Data-Sheet-DS200006368A.pdf > + */ > +#include <linux/bits.h> > +#include <linux/bitfield.h> > +#include <linux/delay.h> > +#include <linux/err.h> > +#include <linux/i2c.h> > +#include <linux/iio/iio.h> > +#include <linux/iio/sysfs.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> > + > +/* Voltage reference, Power-Down control register and DAC Wiperlock status register fields */ > +#define CH_0 GENMASK(1, 0) Prefix with something driver specific as these are too brief and hence high risk of clash with something in a header in future. > +#define CH_1 GENMASK(3, 2) > +#define CH_2 GENMASK(5, 4) > +#define CH_3 GENMASK(7, 6) > +#define CH_4 GENMASK(9, 8) > +#define CH_5 GENMASK(11, 10) > +#define CH_6 GENMASK(13, 12) > +#define CH_7 GENMASK(15, 14) > + > +/* Gain Control and I2C Slave Address Reguster fields */ > +#define G_0 BIT(8) > +#define G_1 BIT(9) > +#define G_2 BIT(10) > +#define G_3 BIT(11) > +#define G_4 BIT(12) > +#define G_5 BIT(13) > +#define G_6 BIT(14) > +#define G_7 BIT(15) > + > +#define MCP47FEB02_GAIN_STATUS_EEWA_MASK BIT(6) > +#define MCP47FEB02_VOLATILE_GAIN_MASK GENMASK(15, 8) > +#define MCP47FEB02_NV_I2C_SLAVE_ADDR_MASK GENMASK(7, 0) > +#define MCP47FEB02_WIPERLOCK_STATUS_MASK GENMASK(15, 0) > +#define CMD_MEM_ADDR_MASK GENMASK(7, 3) > + > +#define CMD_FORMAT(reg, cmd) (FIELD_PREP(CMD_MEM_ADDR_MASK, (reg)) | (cmd)) > +#define READ_CMD 0x06 > +#define WRITE_CMD 0x00 regmap has the ability to set extra bits for reads over writes. Use that and I think you shouldn't need to pass READ_CMD to every read and WRITE_CMD to every write. readflag_mask = 0x6 perhaps does the job. > +#define READ_OP(ch) (((ch) << 3) | READ_CMD) > +#define WRITE_OP(ch) ((ch) << 3) > + > +static const struct regmap_range mcp47feb02_readable_ranges[] = { > + regmap_reg_range(READ_OP(MCP47FEB02_DAC0_REG_ADDR), > + READ_OP(MCP47FEB02_WIPERLOCK_STATUS_REG_ADDR)), > + regmap_reg_range(READ_OP(MCP47FEB02_NV_DAC0_REG_ADDR), > + READ_OP(MCP47FEB02_NV_GAIN_I2C_SLAVE_REG_ADDR)), I might be wrong but that READ_OP looks like you are messing with the addresses rather than handling a protocol that requires setting some flags to indicate a read. I'm not sure how this works with the regcache which would expect reads and writes to end up at the same address. > +}; > + > +static const struct regmap_range mcp47feb02_writable_ranges[] = { > + regmap_reg_range(WRITE_OP(MCP47FEB02_DAC0_REG_ADDR), > + WRITE_OP(MCP47FEB02_WIPERLOCK_STATUS_REG_ADDR)), > + regmap_reg_range(WRITE_OP(MCP47FEB02_NV_DAC0_REG_ADDR), > + WRITE_OP(MCP47FEB02_NV_GAIN_I2C_SLAVE_REG_ADDR)), > +}; > + > +static const struct regmap_config mcp47fvb02_regmap_config = { > + .name = "mcp47fvb02_regmap", > + .reg_bits = 8, > + .val_bits = 16, > + .rd_table = &mcp47fvb02_readable_table, > + .wr_table = &mcp47fvb02_writable_table, > + .volatile_table = &mcp47fvb02_volatile_table, > + .max_register = READ_OP(MCP47FEB02_WIPERLOCK_STATUS_REG_ADDR), > + .cache_type = REGCACHE_RBTREE, use MAPLE for all new code. > +}; > + > +static int mcp47feb02_write_to_register(struct regmap *regmap, unsigned int reg, > + int channel, int tmp_val) > +{ > + int ret, val; > + > + ret = regmap_read(regmap, CMD_FORMAT(reg, READ_CMD), &val); > + if (ret) > + return ret; > + > + if (reg == MCP47FEB02_GAIN_STATUS_REG_ADDR) { > + switch (channel) { > + case 0: > + FIELD_MODIFY(G_0, &val, tmp_val); > + break; > + case 1: > + FIELD_MODIFY(G_1, &val, tmp_val); > + break; > + case 2: > + FIELD_MODIFY(G_2, &val, tmp_val); > + break; > + case 3: > + FIELD_MODIFY(G_3, &val, tmp_val); > + break; > + case 4: > + FIELD_MODIFY(G_4, &val, tmp_val); > + break; > + case 5: > + FIELD_MODIFY(G_5, &val, tmp_val); > + break; > + case 6: > + FIELD_MODIFY(G_6, &val, tmp_val); > + break; > + case 7: > + FIELD_MODIFY(G_7, &val, tmp_val); > + break; > + default: Why default sensible here? Add a comment. > + FIELD_MODIFY(G_0, &val, tmp_val); > + break; > + } > + } else { > + switch (channel) { > + case 0: > + FIELD_MODIFY(CH_0, &val, tmp_val); Sometimes FIELD_MODIFY() and friends are too complex when we are dealing with a bunch of identical sized fields in a register. I'd just code the bit shifts directly. > + break; > + case 1: > + FIELD_MODIFY(CH_1, &val, tmp_val); > + break; > + case 2: > + FIELD_MODIFY(CH_2, &val, tmp_val); > + break; > + case 3: > + FIELD_MODIFY(CH_3, &val, tmp_val); > + break; > + case 4: > + FIELD_MODIFY(CH_4, &val, tmp_val); > + break; > + case 5: > + FIELD_MODIFY(CH_5, &val, tmp_val); > + break; > + case 6: > + FIELD_MODIFY(CH_6, &val, tmp_val); > + break; > + case 7: > + FIELD_MODIFY(CH_7, &val, tmp_val); > + break; > + default: > + FIELD_MODIFY(CH_0, &val, tmp_val); Likewise - add comment on when default applies. > + break; > + } > + } > + > + ret = regmap_write(regmap, CMD_FORMAT(reg, WRITE_CMD), val); > + > + return ret; > +} > + > +static int mcp47feb02_write_to_eeprom(struct regmap *regmap, unsigned int reg, > + unsigned int val) > +{ > + int eewa_val, ret; > + > + /* > + * wait till the currently occurring EEPROM Write Cycle is completed. Wait > + * Only serial commands to the volatile memory are allowed. > + */ > + ret = regmap_read_poll_timeout(regmap, > + CMD_FORMAT(MCP47FEB02_GAIN_STATUS_REG_ADDR, READ_CMD), > + eewa_val, !(eewa_val & MCP47FEB02_GAIN_STATUS_EEWA_MASK), > + MCP47FEB02_DELAY_1_MS, MCP47FEB02_DELAY_1_MS * 5); > + if (ret) > + return ret; > + > + ret = regmap_write(regmap, CMD_FORMAT(reg, WRITE_CMD), val); > + > + return ret; return regmap_write() Check for other simple cases like this. > +} > + > +static ssize_t mcp47feb02_store_eeprom(struct device *dev, struct device_attribute *attr, > + const char *buf, size_t len) > +{ > + struct mcp47feb02_data *data = iio_priv(dev_to_iio_dev(dev)); > + int ret, i, val, val1; > + bool state; > + > + ret = kstrtobool(buf, &state); > + if (ret < 0) > + return ret; > + > + if (!state) > + return 0; > + > + if (!data->have_eeprom) { I assume the attribute doesn't exist so this is just paranoid checking? If so add a comment that you shouldn't be able to get here. > + dev_err(dev, "Device has no eeprom memory.\n"); > + return -EINVAL; > + } > + > + /* > + * Verify DAC Wiper and DAC Configuratioin are unlocked. If both are disabled, > + * writing to EEPROM is available. > + */ > + ret = regmap_read(data->regmap, CMD_FORMAT(MCP47FEB02_WIPERLOCK_STATUS_REG_ADDR, READ_CMD), Wrap shorter to keep nearer to 80 chars where it doesn't hurt readability > + &val); ... > + return len; > +} > + > +static IIO_DEVICE_ATTR(store_eeprom, 0200, NULL, mcp47feb02_store_eeprom, 0); > +static struct attribute *mcp47feb02_attributes[] = { > + &iio_dev_attr_store_eeprom.dev_attr.attr, > + NULL, No comma on a null terminator as we don't want to be able to add stuff after it. > +}; > +static DEFINE_SIMPLE_DEV_PM_OPS(mcp47feb02_pm_ops, mcp47feb02_suspend, mcp47feb02_resume); > + > +static const struct attribute_group mcp47feb02_attribute_group = { > + .attrs = mcp47feb02_attributes, I'd push this near the attribute definition rather than after the PM stuff. > +}; > + > +static int mcp47feb02_set_powerdown_mode(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan, unsigned int mode) > +{ > + struct mcp47feb02_data *data = iio_priv(indio_dev); > + struct device *dev = &data->client->dev; > + int bit_set, ret; > + > + bit_set = test_bit(chan->address, &data->active_channels_mask); > + if (bit_set) { > + ret = mcp47feb02_write_to_register(data->regmap, MCP47FEB02_POWER_DOWN_REG_ADDR, > + chan->address, mode + 1); > + if (ret) > + return ret; > + > + data->chdata[chan->address].powerdown_mode = mode; > + > + return 0; > + } > + > + dev_err(dev, "Channel %ld not enabled\n", chan->address); > + return -EINVAL; > +} > + > +static ssize_t mcp47feb02_read_powerdown(struct iio_dev *indio_dev, uintptr_t private, > + const struct iio_chan_spec *chan, char *buf) > +{ > + struct mcp47feb02_data *data = iio_priv(indio_dev); > + int bit_set; > + > + bit_set = test_bit(chan->address, &data->active_channels_mask); > + if (bit_set) > + return sysfs_emit(buf, "%d\n", data->chdata[chan->address].powerdown); > + > + dev_err(&data->client->dev, "Channel %ld not enabled\n", chan->address); As below. I think this should be impossible in which case no need for the error message. or the sanity check against active_channels_mask. Same for all other cases above. > + return -EINVAL; > +} > + > +static ssize_t mcp47feb02_write_powerdown(struct iio_dev *indio_dev, uintptr_t private, > + const struct iio_chan_spec *chan, const char *buf, > + size_t len) > +{ > + struct mcp47feb02_data *data = iio_priv(indio_dev); > + struct device *dev = &data->client->dev; > + int bit_set, ret; > + bool state; > + > + ret = kstrtobool(buf, &state); > + if (ret) > + return ret; > + > + bit_set = test_bit(chan->address, &data->active_channels_mask); > + if (bit_set) > + data->chdata[chan->address].powerdown = state; > + else > + dev_err(dev, "Channel %ld not enabled\n", chan->address); If it's an error return an error code. Can we actually get here? If we have a powerdown per channel, and the channel isn't there, how is this called? > + > + return len; > +} > + > +static const struct iio_chan_spec_ext_info mcp47feb02_ext_info[] = { > + { > + .name = "powerdown", > + .read = mcp47feb02_read_powerdown, > + .write = mcp47feb02_write_powerdown, > + .shared = IIO_SEPARATE, > + }, > + IIO_ENUM("powerdown_mode", IIO_SEPARATE, &mcp47febxx_powerdown_mode_enum), > + IIO_ENUM_AVAILABLE("powerdown_mode", IIO_SHARED_BY_TYPE, &mcp47febxx_powerdown_mode_enum), > + { }, Trivial but no trailing comma for 'terminating' entrees like this one. > +}; > +static int mcp47feb02_parse_fw(struct iio_dev *indio_dev, const struct mcp47feb02_features *info) > +{ > + struct iio_chan_spec chanspec = mcp47febxx_ch_template; > + struct mcp47feb02_data *data = iio_priv(indio_dev); > + struct device *dev = &data->client->dev; > + struct iio_chan_spec *channels; > + u32 reg, num_channels; > + int chan_idx = 0; > + > + num_channels = device_get_child_node_count(dev); > + if (num_channels > info->phys_channels) > + return dev_err_probe(dev, -EINVAL, "More channels than the chip supports\n"); > + > + channels = devm_kcalloc(dev, num_channels, sizeof(*channels), GFP_KERNEL); > + if (!channels) > + return -ENOMEM; > + > + device_for_each_child_node_scoped(dev, child) { > + fwnode_property_read_u32(child, "reg", ®); Check that for error return. otherwise you just get garbage in reg. Probably want to initialize reg to 0 before calling this anyway to ensure its a consistent value if this fails for some reason. > + > + if (reg >= info->phys_channels) > + return dev_err_probe(dev, -EINVAL, > + "The index of the channels does not match the chip\n"); > + > + set_bit(reg, &data->active_channels_mask); > + > + if (fwnode_property_present(child, "label")) > + fwnode_property_read_string(child, "label", > + (const char **)&data->labels[reg]); > + > + chanspec.address = reg; > + chanspec.channel = reg; > + channels[chan_idx] = chanspec; > + chan_idx++; > + } > + > + indio_dev->num_channels = num_channels; > + indio_dev->channels = channels; > + indio_dev->modes = INDIO_DIRECT_MODE; > + data->phys_channels = info->phys_channels; > + > + /* > + * check if vref-supply, vref1-supply, microchip,vref-buffered and > + * microchip,vref1-buffered are defined in the devicetree > + */ > + data->use_vref = device_property_present(dev, "vref-supply"); > + data->use_vref1 = device_property_present(dev, "vref1-supply"); Just try and get them and handle the regulator errors to discover if they aren't there. However, not all chips have both of these? If not then you should have that info in the chip type specific structures and not try to query them at all if not relevant. > + data->vref_buffered = device_property_read_bool(dev, "microchip,vref-buffered"); > + data->vref1_buffered = device_property_read_bool(dev, "microchip,vref1-buffered"); > + > + return 0; > +} > + > +static int mcp47feb02_probe(struct i2c_client *client) > +{ > + int err, ret, vdd_mv, vref_mv, vref1_mv, i, tmp_vref, vref_ch, gain_ch; > + const struct i2c_device_id *id = i2c_client_get_device_id(client); > + const struct mcp47feb02_features *info; > + enum vref_mode ref_mode, ref_mode1; > + struct device *dev = &client->dev; > + struct mcp47feb02_data *data; > + struct iio_dev *indio_dev; > + > + indio_dev = devm_iio_device_alloc(dev, sizeof(*data)); > + if (!indio_dev) > + return -ENOMEM; > + > + data = iio_priv(indio_dev); > + i2c_set_clientdata(client, indio_dev); > + data->client = client; > + info = i2c_get_match_data(client); > + > + if (info->have_eeprom) { > + data->regmap = devm_regmap_init_i2c(client, &mcp47feb02_regmap_config); > + data->have_eeprom = true; I'd be tempted to stash a const pointer to info inside data. Then you have ready access to the chip specific stuff without leading code to copy things over. > + } else { > + data->regmap = devm_regmap_init_i2c(client, &mcp47fvb02_regmap_config); > + data->have_eeprom = false; > + } > + > + if (IS_ERR(data->regmap)) > + dev_err_probe(dev, PTR_ERR(data->regmap), "Error initializing i2c regmap\n"); > + > + err = mcp47feb02_parse_fw(indio_dev, info); > + if (err) > + return dev_err_probe(dev, err, "Error parsing devicetree data\n"); > + > + if (!info->have_ext_vref2 && data->use_vref1) > + return dev_err_probe(dev, -EINVAL, > + "Second External reference is unavailable on %s\n", > + info->name); > + > + ret = regmap_read(data->regmap, CMD_FORMAT(MCP47FEB02_VREF_REG_ADDR, READ_CMD), &vref_ch); > + if (ret) > + return ret; > + > + ret = regmap_read(data->regmap, CMD_FORMAT(MCP47FEB02_GAIN_STATUS_REG_ADDR, READ_CMD), > + &gain_ch); > + if (ret) > + return ret; > + > + gain_ch = gain_ch >> 8; > + > + /* > + * Values stored in the nonvolatile memory will be transferred to the volatile registers > + * at startup. For safety reasons, the user receives a warning if startup values > + * do not match the ones from current devicetree configuration. > + * Nonvolatile memory can be written at any time > + */ > + for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) { > + /* VDD can be set as Vref only with Gain x1 */ > + if ((vref_ch & 0x03) == MCP47FEB02_VREF_VDD && > + (gain_ch & 0x01) == MCP47FEB02_GAIN_X2) { > + dev_info(dev, "vdd can be used only with gain x1\n"); > + ret = mcp47feb02_write_to_register(data->regmap, > + MCP47FEB02_GAIN_STATUS_REG_ADDR, > + i, MCP47FEB02_GAIN_X1); > + if (ret) > + return ret; > + > + data->chdata[i].use_2x_gain = MCP47FEB02_GAIN_X1; > + } > + > + if (data->phys_channels >= 4 && (i % 2)) { > + if ((vref_ch & 0x03) == MCP47FEB02_EXTERNAL_VREF_BUFFERED && > + data->use_vref1 && !data->vref1_buffered) > + dev_info(dev, "vref1 is unbuffered\n"); This doesn't sound like an error. I'd state that vref1 from eeprom is unbuffered but not from firmware or something like that. > + else if ((vref_ch & 0x03) == MCP47FEB02_EXTERNAL_VREF_UNBUFFERED && > + data->use_vref1 && data->vref1_buffered) > + dev_info(dev, "vref1 is buffered\n"); > + } else { > + if ((vref_ch & 0x03) == MCP47FEB02_EXTERNAL_VREF_BUFFERED && > + data->use_vref && !data->vref_buffered) > + dev_info(dev, "vref is unbuffered\n"); > + else if ((vref_ch & 0x03) == MCP47FEB02_EXTERNAL_VREF_UNBUFFERED && > + data->use_vref && data->vref_buffered) > + dev_info(dev, "vref is buffered\n"); > + } > + > + vref_ch = vref_ch >> 2; > + gain_ch = gain_ch >> 1; > + } > + > + if (data->use_vref) Can we combine all the stuff about a given reference in one place. Probably means dragging the enabling and voltage measurement up here. > + ref_mode = data->vref_buffered ? > + MCP47FEB02_EXTERNAL_VREF_BUFFERED : MCP47FEB02_EXTERNAL_VREF_UNBUFFERED; > + else > + ref_mode = MCP47FEB02_INTERNAL_BAND_GAP; > + > + if (data->use_vref1) > + ref_mode1 = data->vref1_buffered ? > + MCP47FEB02_EXTERNAL_VREF_BUFFERED : MCP47FEB02_EXTERNAL_VREF_UNBUFFERED; > + > + else > + ref_mode1 = MCP47FEB02_INTERNAL_BAND_GAP; > + > + for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) { > + if (data->phys_channels >= 4 && (i % 2)) > + tmp_vref = ref_mode1; > + else > + tmp_vref = ref_mode; > + > + ret = mcp47feb02_write_to_register(data->regmap, MCP47FEB02_VREF_REG_ADDR, > + i, tmp_vref); > + if (ret) > + return ret; > + > + data->chdata[i].ref_mode = tmp_vref; > + } > + > + indio_dev->name = id->name; > + if (info->have_eeprom) > + indio_dev->info = &mcp47feb02_info; > + else > + indio_dev->info = &mcp47fvb02_info; Seems no harm in moving this to where you set the other have_eeprom specific stuff. > + > + ret = devm_mutex_init(dev, &data->lock); > + if (ret < 0) > + return ret; > + > + ret = devm_regulator_get_enable_read_voltage(dev, "vdd"); > + if (ret < 0) > + return ret; > + > + vdd_mv = ret / 1000; > + > + if (data->use_vref) { As above. I would expect to see a check here on whether the chip has the relevant pins (I guess they don't all have two vref?) rather than property presence which is well handled by checking for -ENODEV (I think - check that error code) or is this some risk in turning these on before buffers are enabled? In many boards I'd expect the reference will be hard wired on anyway. > + ret = devm_regulator_get_enable_read_voltage(dev, "vref"); > + if (ret < 0) > + return ret; > + > + vref_mv = ret / 1000; > + } > + > + if (data->use_vref1) { > + ret = devm_regulator_get_enable_read_voltage(dev, "vref1"); > + if (ret < 0) > + return ret; > + > + vref1_mv = ret / 1000; > + } > + > + for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) { > + ret = mcp47feb02_init_scales_avail(info, data, vdd_mv, vref_mv, vref1_mv); Fix the issue Dan found here with uninitialized values. > + if (ret) > + dev_err_probe(dev, ret, "failed to init scales for ch i %d\n", i); If it fails. error out. Or dev_err() isn't appropriate. > + } > + > + err = iio_device_register(indio_dev); > + > + return err; return devm_iio_device_register() > +} > + > +static void mcp47feb02_remove(struct i2c_client *client) > +{ > + struct iio_dev *indio_dev = i2c_get_clientdata(client); > + > + iio_device_unregister(indio_dev); I think someone else covered this, but if nothing else in remove, you should just use devm_iio_device_register() and let devm magic do all your remove path. > +} ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2025-09-29 6:43 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-09-22 11:30 [PATCH 0/2] Adding support for Microchip MCP47FEB02 Ariana Lazar 2025-09-22 11:30 ` [PATCH 1/2] dt-bindings: iio: dac: adding " Ariana Lazar 2025-09-22 16:38 ` Rob Herring (Arm) 2025-09-22 21:59 ` David Lechner 2025-09-27 17:00 ` Jonathan Cameron 2025-09-22 22:00 ` Rob Herring 2025-09-22 11:30 ` [PATCH 2/2] " Ariana Lazar 2025-09-22 20:10 ` Nuno Sá 2025-09-22 22:15 ` David Lechner 2025-09-23 8:21 ` Nuno Sá 2025-09-27 17:13 ` Jonathan Cameron 2025-09-29 5:44 ` Nuno Sá 2025-09-26 15:38 ` Dan Carpenter 2025-09-27 17:53 ` Jonathan Cameron
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).