* [PATCH 0/2] Add support for Microchip MCP48FxBy1/2/4/8 DAC with an SPI Interface
@ 2026-02-12 12:48 Ariana Lazar
2026-02-12 12:48 ` [PATCH 1/2] dt-bindings: iio: dac: add support for Microchip MCP48FEB02 Ariana Lazar
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: Ariana Lazar @ 2026-02-12 12:48 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
Add support for Microchip MCP48FxBy1/2/4/8 series of buffered voltage
output Digital-to-Analog converters with an SPI Interface. This driver
covers the following part numbers:
- With nonvolatile memory:
- MCP48FEB01, MCP48FEB02, MCP48FEB04, MCP48FEB08,
MCP48FEB11, MCP48FEB12, MCP48FEB14, MCP48FEB18,
MCP48FEB21, MCP48FEB22, MCP48FEB24, MCP48FEB28
- With volatile memory:
- MCP48FVB01, MCP48FVB02, MCP48FVB04, MCP48FVB08,
MCP48FVB11, MCP48FVB12, MCP48FVB14, MCP48FVB18,
MCP48FVB21, MCP48FVB22, MCP48FVB24, MCP48FVB28
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: add support for Microchip MCP48FEB02
iio: dac: add support for Microchip MCP48FEB02
.../bindings/iio/dac/microchip,mcp48feb02.yaml | 299 +++++
MAINTAINERS | 7 +
drivers/iio/dac/Kconfig | 20 +
drivers/iio/dac/Makefile | 1 +
drivers/iio/dac/mcp48feb02.c | 1243 ++++++++++++++++++++
5 files changed, 1570 insertions(+)
---
base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
change-id: 20260115-mcp48feb02-4d9f17811660
Best regards,
--
Ariana Lazar <ariana.lazar@microchip.com>
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 1/2] dt-bindings: iio: dac: add support for Microchip MCP48FEB02 2026-02-12 12:48 [PATCH 0/2] Add support for Microchip MCP48FxBy1/2/4/8 DAC with an SPI Interface Ariana Lazar @ 2026-02-12 12:48 ` Ariana Lazar 2026-02-12 17:31 ` Rob Herring (Arm) 2026-02-12 18:00 ` Conor Dooley 2026-02-12 12:48 ` [PATCH 2/2] " Ariana Lazar 2026-02-12 13:39 ` [PATCH 0/2] Add support for Microchip MCP48FxBy1/2/4/8 DAC with an SPI Interface Andy Shevchenko 2 siblings, 2 replies; 18+ messages in thread From: Ariana Lazar @ 2026-02-12 12:48 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 MCP48FxBy1/2/4/8 series of buffered voltage output Digital-to-Analog Converters with nonvolatile or volatile memory and an SPI 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,mcp48feb02.yaml | 299 +++++++++++++++++++++ MAINTAINERS | 6 + 2 files changed, 305 insertions(+) diff --git a/Documentation/devicetree/bindings/iio/dac/microchip,mcp48feb02.yaml b/Documentation/devicetree/bindings/iio/dac/microchip,mcp48feb02.yaml new file mode 100644 index 0000000000000000000000000000000000000000..78c6bd641c6e37321e4fc056db83eb4277f429b8 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/dac/microchip,mcp48feb02.yaml @@ -0,0 +1,299 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/dac/microchip,mcp48feb02.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Microchip MCP48F(E/V)B(0/1/2)(1/2/4/8) DAC with SPI Interface Families + +maintainers: + - Ariana Lazar <ariana.lazar@microchip.com> + +description: | + Datasheet for MCP48FEB01, MCP48FEB02, MCP48FEB11, MCP48FEB12, MCP48FEB21, + MCP48FEB22 can be found here: + https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005429B.pdf + Datasheet for MCP48FVB01, MCP48FVB02, MCP48FVB11, MCP48FVB12, MCP48FVB21, + MCP48FVB22 can be found here: + https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005466A.pdf + Datasheet for MCP48FEB04, MCP48FEB14, MCP48FEB24, MCP48FEB08, MCP48FEB18, + MCP48FEB28, MCP48FVB04, MCP48FVB14, MCP48FVB24, MCP48FVB08, MCP48FVB18, + MCP48FVB28 can be found here: + https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/MCP48FXBX4-8-Family-Data-Sheet-DS20006362A.pdf + + +------------+--------------+-------------+-------------+------------+ + | Device | Resolution | Channels | Vref number | Memory | + |------------|--------------|-------------|-------------|------------| + | MCP48FEB01 | | 1 | 1 | EEPROM | + | MCP48FEB02 | 8-bit | 2 | 1 | EEPROM | + | MCP48FEB04 | | 4 | 2 | EEPROM | + | MCP48FEB08 | | 8 | 2 | EEPROM | + |------------|--------------|-------------|-------------|------------| + | MCP48FEB11 | | 1 | 1 | EEPROM | + | MCP48FEB12 | 10-bit | 2 | 1 | EEPROM | + | MCP48FEB14 | | 4 | 2 | EEPROM | + | MCP48FEB18 | | 8 | 2 | EEPROM | + |------------|--------------|-------------|-------------|------------| + | MCP48FEB21 | | 1 | 1 | EEPROM | + | MCP48FEB22 | 12-bit | 2 | 1 | EEPROM | + | MCP48FEB24 | | 4 | 2 | EEPROM | + | MCP48FEB28 | | 8 | 2 | EEPROM | + |------------|--------------|-------------|-------------|------------| + | MCP48FVB01 | | 1 | 1 | RAM | + | MCP48FVB02 | 8-bit | 2 | 1 | RAM | + | MCP48FVB04 | | 4 | 2 | RAM | + | MCP48FVB08 | | 8 | 2 | RAM | + |------------|--------------|-------------|-------------|------------| + | MCP48FVB11 | | 1 | 1 | RAM | + | MCP48FVB12 | 10-bit | 2 | 1 | RAM | + | MCP48FVB14 | | 4 | 2 | RAM | + | MCP48FVB18 | | 8 | 2 | RAM | + |------------|--------------|-------------|-------------|------------| + | MCP48FVB21 | | 1 | 1 | RAM | + | MCP48FVB22 | 12-bit | 2 | 1 | RAM | + | MCP48FVB24 | | 4 | 2 | RAM | + | MCP48FVB28 | | 8 | 2 | RAM | + +------------+--------------+-------------+-------------+------------+ + +properties: + compatible: + enum: + - microchip,mcp48feb01 + - microchip,mcp48feb02 + - microchip,mcp48feb04 + - microchip,mcp48feb08 + - microchip,mcp48feb11 + - microchip,mcp48feb12 + - microchip,mcp48feb14 + - microchip,mcp48feb18 + - microchip,mcp48feb21 + - microchip,mcp48feb22 + - microchip,mcp48feb24 + - microchip,mcp48feb28 + - microchip,mcp48fvb01 + - microchip,mcp48fvb02 + - microchip,mcp48fvb04 + - microchip,mcp48fvb08 + - microchip,mcp48fvb11 + - microchip,mcp48fvb12 + - microchip,mcp48fvb14 + - microchip,mcp48fvb18 + - microchip,mcp48fvb21 + - microchip,mcp48fvb22 + - microchip,mcp48fvb24 + - microchip,mcp48fvb28 + + reg: + maxItems: 1 + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + + vdd-supply: + description: + Provides power to the chip and it could be used as reference voltage. The + voltage is used to calculate scale. For parts without EEPROM at powerup + this will be the selected as voltage reference. + + vref-supply: + description: | + Vref pin (it could be found as Vref0 into the datasheet) may be used as a + voltage reference when this supply is specified. The internal reference + will be taken into account for voltage reference besides VDD if this supply + does not exist. + + This supply will be voltage reference 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, Vout4, Vout6; + + vref1-supply: + description: | + Vref1 pin may be used as a voltage reference when this supply is specified. + The internal reference will be taken into account for voltage reference + beside VDD if this supply does not exist. + + This supply will be voltage reference 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. 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 its 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 its voltage when connected to the internal resistor ladder + circuit. + +patternProperties: + "^channel@[0-7]$": + $ref: dac.yaml + type: object + description: Voltage output channel. + + properties: + reg: + description: The channel number. + maxItems: 1 + + 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,mcp48feb01 + - microchip,mcp48feb11 + - microchip,mcp48feb21 + - microchip,mcp48fvb01 + - microchip,mcp48fvb11 + - microchip,mcp48fvb21 + then: + properties: + lat1-gpios: false + vref1-supply: false + microchip,vref1-buffered: false + channel@0: + properties: + reg: + const: 0 + patternProperties: + "^channel@[1-7]$": false + - if: + properties: + compatible: + contains: + enum: + - microchip,mcp48feb02 + - microchip,mcp48feb12 + - microchip,mcp48feb22 + - microchip,mcp48fvb02 + - microchip,mcp48fvb12 + - microchip,mcp48fvb22 + then: + properties: + lat1-gpios: false + vref1-supply: false + microchip,vref1-buffered: false + patternProperties: + "^channel@[0-1]$": + properties: + reg: + enum: [0, 1] + "^channel@[2-7]$": false + - if: + properties: + compatible: + contains: + enum: + - microchip,mcp48fvb04 + - microchip,mcp48fvb14 + - microchip,mcp48fvb24 + - microchip,mcp48feb04 + - microchip,mcp48feb14 + - microchip,mcp48feb24 + then: + patternProperties: + "^channel@[0-3]$": + properties: + reg: + enum: [0, 1, 2, 3] + "^channel@[4-7]$": false + - if: + properties: + compatible: + contains: + enum: + - microchip,mcp48fvb08 + - microchip,mcp48fvb18 + - microchip,mcp48fvb28 + - microchip,mcp48feb08 + - microchip,mcp48feb18 + - microchip,mcp48feb28 + then: + patternProperties: + "^channel@[0-7]$": + properties: + reg: + enum: [0, 1, 2, 3, 4, 5, 6, 7] + - 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: + - | + spi { + #address-cells = <1>; + #size-cells = <0>; + + dac@0 { + compatible = "microchip,mcp48feb08"; + reg = <0>; + vdd-supply = <&vdac_vdd>; + vref-supply = <&vref_reg>; + + #address-cells = <1>; + #size-cells = <0>; + channel@0 { + reg = <0>; + label = "Adjustable_voltage_ch0"; + }; + + channel@1 { + reg = <0x1>; + label = "Adjustable_voltage_ch1"; + }; + }; + }; +... diff --git a/MAINTAINERS b/MAINTAINERS index a92290fffa163f9fe8fe3f04bf66426f9a894409..ed24fd2758ad0103dbc5191d0ec180f8ee5e8298 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14945,6 +14945,12 @@ S: Maintained F: Documentation/devicetree/bindings/iio/dac/microchip,mcp4821.yaml F: drivers/iio/dac/mcp4821.c +MCP48FEB02 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,mcp48feb02.yaml + MCR20A IEEE-802.15.4 RADIO DRIVER M: Stefan Schmidt <stefan@datenfreihafen.org> L: linux-wpan@vger.kernel.org -- 2.43.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: dac: add support for Microchip MCP48FEB02 2026-02-12 12:48 ` [PATCH 1/2] dt-bindings: iio: dac: add support for Microchip MCP48FEB02 Ariana Lazar @ 2026-02-12 17:31 ` Rob Herring (Arm) 2026-02-12 18:00 ` Conor Dooley 1 sibling, 0 replies; 18+ messages in thread From: Rob Herring (Arm) @ 2026-02-12 17:31 UTC (permalink / raw) To: Ariana Lazar Cc: linux-iio, Nuno Sá, linux-kernel, Conor Dooley, Andy Shevchenko, Jonathan Cameron, Krzysztof Kozlowski, David Lechner, devicetree On Thu, 12 Feb 2026 14:48:34 +0200, Ariana Lazar wrote: > This is the device tree schema for iio driver for Microchip > MCP48FxBy1/2/4/8 series of buffered voltage output Digital-to-Analog > Converters with nonvolatile or volatile memory and an SPI 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,mcp48feb02.yaml | 299 +++++++++++++++++++++ > MAINTAINERS | 6 + > 2 files changed, 305 insertions(+) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: ./Documentation/devicetree/bindings/iio/dac/microchip,mcp48feb02.yaml:275:5: [warning] wrong indentation: expected 2 but found 4 (indentation) dtschema/dtc warnings/errors: doc reference errors (make refcheckdocs): See https://patchwork.kernel.org/project/devicetree/patch/20260212-mcp48feb02-v1-1-ce5843db65db@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] 18+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: dac: add support for Microchip MCP48FEB02 2026-02-12 12:48 ` [PATCH 1/2] dt-bindings: iio: dac: add support for Microchip MCP48FEB02 Ariana Lazar 2026-02-12 17:31 ` Rob Herring (Arm) @ 2026-02-12 18:00 ` Conor Dooley 2026-02-12 20:04 ` Andy Shevchenko 1 sibling, 1 reply; 18+ messages in thread From: Conor Dooley @ 2026-02-12 18:00 UTC (permalink / raw) To: Ariana Lazar Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 13024 bytes --] On Thu, Feb 12, 2026 at 02:48:34PM +0200, Ariana Lazar wrote: > This is the device tree schema for iio driver for Microchip > MCP48FxBy1/2/4/8 series of buffered voltage output Digital-to-Analog > Converters with nonvolatile or volatile memory and an SPI 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> Other than the interface, what's actually different between this and the 47? Could they share the same binding? Cheers, Conor. > --- > .../bindings/iio/dac/microchip,mcp48feb02.yaml | 299 +++++++++++++++++++++ > MAINTAINERS | 6 + > 2 files changed, 305 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iio/dac/microchip,mcp48feb02.yaml b/Documentation/devicetree/bindings/iio/dac/microchip,mcp48feb02.yaml > new file mode 100644 > index 0000000000000000000000000000000000000000..78c6bd641c6e37321e4fc056db83eb4277f429b8 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/dac/microchip,mcp48feb02.yaml > @@ -0,0 +1,299 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/dac/microchip,mcp48feb02.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Microchip MCP48F(E/V)B(0/1/2)(1/2/4/8) DAC with SPI Interface Families > + > +maintainers: > + - Ariana Lazar <ariana.lazar@microchip.com> > + > +description: | > + Datasheet for MCP48FEB01, MCP48FEB02, MCP48FEB11, MCP48FEB12, MCP48FEB21, > + MCP48FEB22 can be found here: > + https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005429B.pdf > + Datasheet for MCP48FVB01, MCP48FVB02, MCP48FVB11, MCP48FVB12, MCP48FVB21, > + MCP48FVB22 can be found here: > + https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005466A.pdf > + Datasheet for MCP48FEB04, MCP48FEB14, MCP48FEB24, MCP48FEB08, MCP48FEB18, > + MCP48FEB28, MCP48FVB04, MCP48FVB14, MCP48FVB24, MCP48FVB08, MCP48FVB18, > + MCP48FVB28 can be found here: > + https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/MCP48FXBX4-8-Family-Data-Sheet-DS20006362A.pdf > + > + +------------+--------------+-------------+-------------+------------+ > + | Device | Resolution | Channels | Vref number | Memory | > + |------------|--------------|-------------|-------------|------------| > + | MCP48FEB01 | | 1 | 1 | EEPROM | > + | MCP48FEB02 | 8-bit | 2 | 1 | EEPROM | > + | MCP48FEB04 | | 4 | 2 | EEPROM | > + | MCP48FEB08 | | 8 | 2 | EEPROM | > + |------------|--------------|-------------|-------------|------------| > + | MCP48FEB11 | | 1 | 1 | EEPROM | > + | MCP48FEB12 | 10-bit | 2 | 1 | EEPROM | > + | MCP48FEB14 | | 4 | 2 | EEPROM | > + | MCP48FEB18 | | 8 | 2 | EEPROM | > + |------------|--------------|-------------|-------------|------------| > + | MCP48FEB21 | | 1 | 1 | EEPROM | > + | MCP48FEB22 | 12-bit | 2 | 1 | EEPROM | > + | MCP48FEB24 | | 4 | 2 | EEPROM | > + | MCP48FEB28 | | 8 | 2 | EEPROM | > + |------------|--------------|-------------|-------------|------------| > + | MCP48FVB01 | | 1 | 1 | RAM | > + | MCP48FVB02 | 8-bit | 2 | 1 | RAM | > + | MCP48FVB04 | | 4 | 2 | RAM | > + | MCP48FVB08 | | 8 | 2 | RAM | > + |------------|--------------|-------------|-------------|------------| > + | MCP48FVB11 | | 1 | 1 | RAM | > + | MCP48FVB12 | 10-bit | 2 | 1 | RAM | > + | MCP48FVB14 | | 4 | 2 | RAM | > + | MCP48FVB18 | | 8 | 2 | RAM | > + |------------|--------------|-------------|-------------|------------| > + | MCP48FVB21 | | 1 | 1 | RAM | > + | MCP48FVB22 | 12-bit | 2 | 1 | RAM | > + | MCP48FVB24 | | 4 | 2 | RAM | > + | MCP48FVB28 | | 8 | 2 | RAM | > + +------------+--------------+-------------+-------------+------------+ > + > +properties: > + compatible: > + enum: > + - microchip,mcp48feb01 > + - microchip,mcp48feb02 > + - microchip,mcp48feb04 > + - microchip,mcp48feb08 > + - microchip,mcp48feb11 > + - microchip,mcp48feb12 > + - microchip,mcp48feb14 > + - microchip,mcp48feb18 > + - microchip,mcp48feb21 > + - microchip,mcp48feb22 > + - microchip,mcp48feb24 > + - microchip,mcp48feb28 > + - microchip,mcp48fvb01 > + - microchip,mcp48fvb02 > + - microchip,mcp48fvb04 > + - microchip,mcp48fvb08 > + - microchip,mcp48fvb11 > + - microchip,mcp48fvb12 > + - microchip,mcp48fvb14 > + - microchip,mcp48fvb18 > + - microchip,mcp48fvb21 > + - microchip,mcp48fvb22 > + - microchip,mcp48fvb24 > + - microchip,mcp48fvb28 > + > + reg: > + maxItems: 1 > + > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > + vdd-supply: > + description: > + Provides power to the chip and it could be used as reference voltage. The > + voltage is used to calculate scale. For parts without EEPROM at powerup > + this will be the selected as voltage reference. > + > + vref-supply: > + description: | > + Vref pin (it could be found as Vref0 into the datasheet) may be used as a > + voltage reference when this supply is specified. The internal reference > + will be taken into account for voltage reference besides VDD if this supply > + does not exist. > + > + This supply will be voltage reference 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, Vout4, Vout6; > + > + vref1-supply: > + description: | > + Vref1 pin may be used as a voltage reference when this supply is specified. > + The internal reference will be taken into account for voltage reference > + beside VDD if this supply does not exist. > + > + This supply will be voltage reference 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. 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 its 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 its voltage when connected to the internal resistor ladder > + circuit. > + > +patternProperties: > + "^channel@[0-7]$": > + $ref: dac.yaml > + type: object > + description: Voltage output channel. > + > + properties: > + reg: > + description: The channel number. > + maxItems: 1 > + > + 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,mcp48feb01 > + - microchip,mcp48feb11 > + - microchip,mcp48feb21 > + - microchip,mcp48fvb01 > + - microchip,mcp48fvb11 > + - microchip,mcp48fvb21 > + then: > + properties: > + lat1-gpios: false > + vref1-supply: false > + microchip,vref1-buffered: false > + channel@0: > + properties: > + reg: > + const: 0 > + patternProperties: > + "^channel@[1-7]$": false > + - if: > + properties: > + compatible: > + contains: > + enum: > + - microchip,mcp48feb02 > + - microchip,mcp48feb12 > + - microchip,mcp48feb22 > + - microchip,mcp48fvb02 > + - microchip,mcp48fvb12 > + - microchip,mcp48fvb22 > + then: > + properties: > + lat1-gpios: false > + vref1-supply: false > + microchip,vref1-buffered: false > + patternProperties: > + "^channel@[0-1]$": > + properties: > + reg: > + enum: [0, 1] > + "^channel@[2-7]$": false > + - if: > + properties: > + compatible: > + contains: > + enum: > + - microchip,mcp48fvb04 > + - microchip,mcp48fvb14 > + - microchip,mcp48fvb24 > + - microchip,mcp48feb04 > + - microchip,mcp48feb14 > + - microchip,mcp48feb24 > + then: > + patternProperties: > + "^channel@[0-3]$": > + properties: > + reg: > + enum: [0, 1, 2, 3] > + "^channel@[4-7]$": false > + - if: > + properties: > + compatible: > + contains: > + enum: > + - microchip,mcp48fvb08 > + - microchip,mcp48fvb18 > + - microchip,mcp48fvb28 > + - microchip,mcp48feb08 > + - microchip,mcp48feb18 > + - microchip,mcp48feb28 > + then: > + patternProperties: > + "^channel@[0-7]$": > + properties: > + reg: > + enum: [0, 1, 2, 3, 4, 5, 6, 7] > + - 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: > + - | > + spi { > + #address-cells = <1>; > + #size-cells = <0>; > + > + dac@0 { > + compatible = "microchip,mcp48feb08"; > + reg = <0>; > + vdd-supply = <&vdac_vdd>; > + vref-supply = <&vref_reg>; > + > + #address-cells = <1>; > + #size-cells = <0>; > + channel@0 { > + reg = <0>; > + label = "Adjustable_voltage_ch0"; > + }; > + > + channel@1 { > + reg = <0x1>; > + label = "Adjustable_voltage_ch1"; > + }; > + }; > + }; > +... > diff --git a/MAINTAINERS b/MAINTAINERS > index a92290fffa163f9fe8fe3f04bf66426f9a894409..ed24fd2758ad0103dbc5191d0ec180f8ee5e8298 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -14945,6 +14945,12 @@ S: Maintained > F: Documentation/devicetree/bindings/iio/dac/microchip,mcp4821.yaml > F: drivers/iio/dac/mcp4821.c > > +MCP48FEB02 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,mcp48feb02.yaml > + > MCR20A IEEE-802.15.4 RADIO DRIVER > M: Stefan Schmidt <stefan@datenfreihafen.org> > L: linux-wpan@vger.kernel.org > > -- > 2.43.0 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: dac: add support for Microchip MCP48FEB02 2026-02-12 18:00 ` Conor Dooley @ 2026-02-12 20:04 ` Andy Shevchenko 2026-02-16 13:31 ` Ariana.Lazar 0 siblings, 1 reply; 18+ messages in thread From: Andy Shevchenko @ 2026-02-12 20:04 UTC (permalink / raw) To: Conor Dooley Cc: Ariana Lazar, Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio, devicetree, linux-kernel On Thu, Feb 12, 2026 at 06:00:06PM +0000, Conor Dooley wrote: > On Thu, Feb 12, 2026 at 02:48:34PM +0200, Ariana Lazar wrote: > > This is the device tree schema for iio driver for Microchip > > MCP48FxBy1/2/4/8 series of buffered voltage output Digital-to-Analog > > Converters with nonvolatile or volatile memory and an SPI 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> > > Other than the interface, what's actually different between this and the > 47? Could they share the same binding? If that is the case, I don't think we even need a brand new driver, the existing one should be refactored to adapt SPI interface. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: dac: add support for Microchip MCP48FEB02 2026-02-12 20:04 ` Andy Shevchenko @ 2026-02-16 13:31 ` Ariana.Lazar 2026-02-16 15:37 ` David Lechner 0 siblings, 1 reply; 18+ messages in thread From: Ariana.Lazar @ 2026-02-16 13:31 UTC (permalink / raw) To: conor, andriy.shevchenko Cc: dlechner, nuno.sa, linux-iio, devicetree, robh, jic23, andy, krzk+dt, linux-kernel, conor+dt Hi all, Thank you for your reviews. On Thu, 2026-02-12 at 22:04 +0200, Andy Shevchenko wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > > On Thu, Feb 12, 2026 at 06:00:06PM +0000, Conor Dooley wrote: > > On Thu, Feb 12, 2026 at 02:48:34PM +0200, Ariana Lazar wrote: > > > This is the device tree schema for iio driver for Microchip > > > MCP48FxBy1/2/4/8 series of buffered voltage output Digital-to- > > > Analog > > > Converters with nonvolatile or volatile memory and an SPI > > > 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> > > > > Other than the interface, what's actually different between this > > and the > > 47? Could they share the same binding? > > If that is the case, I don't think we even need a brand new driver, > the > existing one should be refactored to adapt SPI interface. > > -- > With Best Regards, > Andy Shevchenko > > I have decided to submit two separate drivers, even though the chips share similar functionality, in order to make it easier for the client to identify the supported chips. For example the I2C family of devices has: 3 different resolutions, with 4 different channel numbers available for a particular part and most important you can get the same part with or without EEPROM. That means the I2C driver will cover 24 different devices. The SPI family follows the same pattern, covering another 24 devices. Microchip also has some devices (I2C and SPI) with Nonvolatile Memory (similar to EEPROM but limited to fewer than 32 writes) and I want to add these families to the existing drivers while maintaining the split by interface. Please tell me if you have anything against this approach (having 2 different drivers split based on interface and each of them to support at least 24 different part numbers). Best regards, Ariana ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: dac: add support for Microchip MCP48FEB02 2026-02-16 13:31 ` Ariana.Lazar @ 2026-02-16 15:37 ` David Lechner 2026-02-16 17:34 ` Conor Dooley 0 siblings, 1 reply; 18+ messages in thread From: David Lechner @ 2026-02-16 15:37 UTC (permalink / raw) To: Ariana.Lazar, conor, andriy.shevchenko Cc: nuno.sa, linux-iio, devicetree, robh, jic23, andy, krzk+dt, linux-kernel, conor+dt On 2/16/26 7:31 AM, Ariana.Lazar@microchip.com wrote: > Hi all, > > Thank you for your reviews. > > > On Thu, 2026-02-12 at 22:04 +0200, Andy Shevchenko wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you >> know the content is safe >> >> On Thu, Feb 12, 2026 at 06:00:06PM +0000, Conor Dooley wrote: >>> On Thu, Feb 12, 2026 at 02:48:34PM +0200, Ariana Lazar wrote: >>>> This is the device tree schema for iio driver for Microchip >>>> MCP48FxBy1/2/4/8 series of buffered voltage output Digital-to- >>>> Analog >>>> Converters with nonvolatile or volatile memory and an SPI >>>> 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> >>> >>> Other than the interface, what's actually different between this >>> and the >>> 47? Could they share the same binding? >> >> If that is the case, I don't think we even need a brand new driver, >> the >> existing one should be refactored to adapt SPI interface. >> >> -- >> With Best Regards, >> Andy Shevchenko >> >> > > > I have decided to submit two separate drivers, even though the chips > share similar functionality, in order to make it easier for the client > to identify the supported chips. > > For example the I2C family of devices has: 3 different resolutions, > with 4 different channel numbers available for a particular part and > most important you can get the same part with or without EEPROM. > That means the I2C driver will cover 24 different devices. The SPI > family follows the same pattern, covering another 24 devices. > > Microchip also has some devices (I2C and SPI) with Nonvolatile Memory > (similar to EEPROM but limited to fewer than 32 writes) and I want to > add these families to the existing drivers while maintaining the split > by interface. > > Please tell me if you have anything against this approach (having 2 > different drivers split based on interface and each of them to support > at least 24 different part numbers). > > Best regards, > Ariana The usual way we support parts with the same register map that can have an I2C or a SPI bus it to make three modules: <name>_core.c, <name>_i2c.c and <name>_spi.c. If you look through the iio folders, you will see many drivers like this. The _i2c.c and _spi.c files will just contain the chip info tables that contain all of the differences between the chips and pass that to a common probe function in the _core.c module. It seems like this approach should work in your case as well. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: dac: add support for Microchip MCP48FEB02 2026-02-16 15:37 ` David Lechner @ 2026-02-16 17:34 ` Conor Dooley 2026-02-16 18:38 ` David Lechner 0 siblings, 1 reply; 18+ messages in thread From: Conor Dooley @ 2026-02-16 17:34 UTC (permalink / raw) To: David Lechner Cc: Ariana.Lazar, andriy.shevchenko, nuno.sa, linux-iio, devicetree, robh, jic23, andy, krzk+dt, linux-kernel, conor+dt [-- Attachment #1: Type: text/plain, Size: 3050 bytes --] On Mon, Feb 16, 2026 at 09:37:35AM -0600, David Lechner wrote: > On 2/16/26 7:31 AM, Ariana.Lazar@microchip.com wrote: > > Hi all, > > > > Thank you for your reviews. > > > > > > On Thu, 2026-02-12 at 22:04 +0200, Andy Shevchenko wrote: > >> EXTERNAL EMAIL: Do not click links or open attachments unless you > >> know the content is safe > >> > >> On Thu, Feb 12, 2026 at 06:00:06PM +0000, Conor Dooley wrote: > >>> On Thu, Feb 12, 2026 at 02:48:34PM +0200, Ariana Lazar wrote: > >>>> This is the device tree schema for iio driver for Microchip > >>>> MCP48FxBy1/2/4/8 series of buffered voltage output Digital-to- > >>>> Analog > >>>> Converters with nonvolatile or volatile memory and an SPI > >>>> 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> > >>> > >>> Other than the interface, what's actually different between this > >>> and the > >>> 47? Could they share the same binding? > >> > >> If that is the case, I don't think we even need a brand new driver, > >> the > >> existing one should be refactored to adapt SPI interface. > >> > >> -- > >> With Best Regards, > >> Andy Shevchenko > >> > >> > > > > > > I have decided to submit two separate drivers, even though the chips > > share similar functionality, in order to make it easier for the client > > to identify the supported chips. > > > > For example the I2C family of devices has: 3 different resolutions, > > with 4 different channel numbers available for a particular part and > > most important you can get the same part with or without EEPROM. > > That means the I2C driver will cover 24 different devices. The SPI > > family follows the same pattern, covering another 24 devices. > > > > Microchip also has some devices (I2C and SPI) with Nonvolatile Memory > > (similar to EEPROM but limited to fewer than 32 writes) and I want to > > add these families to the existing drivers while maintaining the split > > by interface. > > > > Please tell me if you have anything against this approach (having 2 > > different drivers split based on interface and each of them to support > > at least 24 different part numbers). > > > > Best regards, > > Ariana > > The usual way we support parts with the same register map that can have > an I2C or a SPI bus it to make three modules: <name>_core.c, <name>_i2c.c > and <name>_spi.c. If you look through the iio folders, you will see many > drivers like this. > > The _i2c.c and _spi.c files will just contain the chip info tables that > contain all of the differences between the chips and pass that to a > common probe function in the _core.c module. > > It seems like this approach should work in your case as well. These usually have merged bindings too, right? Only real difference is going to be that the spi devices will need spi-peripheral-properties.yaml which obviously the i2c ones wont. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: dac: add support for Microchip MCP48FEB02 2026-02-16 17:34 ` Conor Dooley @ 2026-02-16 18:38 ` David Lechner 0 siblings, 0 replies; 18+ messages in thread From: David Lechner @ 2026-02-16 18:38 UTC (permalink / raw) To: Conor Dooley Cc: Ariana.Lazar, andriy.shevchenko, nuno.sa, linux-iio, devicetree, robh, jic23, andy, krzk+dt, linux-kernel, conor+dt On 2/16/26 11:34 AM, Conor Dooley wrote: > On Mon, Feb 16, 2026 at 09:37:35AM -0600, David Lechner wrote: >> On 2/16/26 7:31 AM, Ariana.Lazar@microchip.com wrote: >>> Hi all, >>> >>> Thank you for your reviews. >>> >>> >>> On Thu, 2026-02-12 at 22:04 +0200, Andy Shevchenko wrote: >>>> EXTERNAL EMAIL: Do not click links or open attachments unless you >>>> know the content is safe >>>> >>>> On Thu, Feb 12, 2026 at 06:00:06PM +0000, Conor Dooley wrote: >>>>> On Thu, Feb 12, 2026 at 02:48:34PM +0200, Ariana Lazar wrote: >>>>>> This is the device tree schema for iio driver for Microchip >>>>>> MCP48FxBy1/2/4/8 series of buffered voltage output Digital-to- >>>>>> Analog >>>>>> Converters with nonvolatile or volatile memory and an SPI >>>>>> 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> >>>>> >>>>> Other than the interface, what's actually different between this >>>>> and the >>>>> 47? Could they share the same binding? >>>> >>>> If that is the case, I don't think we even need a brand new driver, >>>> the >>>> existing one should be refactored to adapt SPI interface. >>>> >>>> -- >>>> With Best Regards, >>>> Andy Shevchenko >>>> >>>> >>> >>> >>> I have decided to submit two separate drivers, even though the chips >>> share similar functionality, in order to make it easier for the client >>> to identify the supported chips. >>> >>> For example the I2C family of devices has: 3 different resolutions, >>> with 4 different channel numbers available for a particular part and >>> most important you can get the same part with or without EEPROM. >>> That means the I2C driver will cover 24 different devices. The SPI >>> family follows the same pattern, covering another 24 devices. >>> >>> Microchip also has some devices (I2C and SPI) with Nonvolatile Memory >>> (similar to EEPROM but limited to fewer than 32 writes) and I want to >>> add these families to the existing drivers while maintaining the split >>> by interface. >>> >>> Please tell me if you have anything against this approach (having 2 >>> different drivers split based on interface and each of them to support >>> at least 24 different part numbers). >>> >>> Best regards, >>> Ariana >> >> The usual way we support parts with the same register map that can have >> an I2C or a SPI bus it to make three modules: <name>_core.c, <name>_i2c.c >> and <name>_spi.c. If you look through the iio folders, you will see many >> drivers like this. >> >> The _i2c.c and _spi.c files will just contain the chip info tables that >> contain all of the differences between the chips and pass that to a >> common probe function in the _core.c module. >> >> It seems like this approach should work in your case as well. > > These usually have merged bindings too, right? Only real difference is > going to be that the spi devices will need > spi-peripheral-properties.yaml which obviously the i2c ones wont. Correct. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/2] iio: dac: add support for Microchip MCP48FEB02 2026-02-12 12:48 [PATCH 0/2] Add support for Microchip MCP48FxBy1/2/4/8 DAC with an SPI Interface Ariana Lazar 2026-02-12 12:48 ` [PATCH 1/2] dt-bindings: iio: dac: add support for Microchip MCP48FEB02 Ariana Lazar @ 2026-02-12 12:48 ` Ariana Lazar 2026-02-12 14:42 ` Andy Shevchenko 2026-02-15 17:58 ` Jonathan Cameron 2026-02-12 13:39 ` [PATCH 0/2] Add support for Microchip MCP48FxBy1/2/4/8 DAC with an SPI Interface Andy Shevchenko 2 siblings, 2 replies; 18+ messages in thread From: Ariana Lazar @ 2026-02-12 12:48 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 MCP48FxBy1/2/4/8 series of buffered voltage output Digital-to-Analog Converters with nonvolatile or volatile memory and an SPI 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 | 20 + drivers/iio/dac/Makefile | 1 + drivers/iio/dac/mcp48feb02.c | 1243 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 1265 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index ed24fd2758ad0103dbc5191d0ec180f8ee5e8298..c861001ff9a94af2ff98c9c697e40909a4c69f6d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14950,6 +14950,7 @@ M: Ariana Lazar <ariana.lazar@microchip.com> L: linux-iio@vger.kernel.org S: Supported F: Documentation/devicetree/bindings/iio/dac/microchip,mcp48feb02.yaml +F: drivers/iio/dac/mcp48feb02.c MCR20A IEEE-802.15.4 RADIO DRIVER M: Stefan Schmidt <stefan@datenfreihafen.org> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig index e0996dc014a3d538ab6b4e0d50ff54ede50f1527..56fb14abb36c6955348ca0b1ad72ce47a4af4e0d 100644 --- a/drivers/iio/dac/Kconfig +++ b/drivers/iio/dac/Kconfig @@ -519,6 +519,26 @@ config MCP4821 To compile this driver as a module, choose M here: the module will be called mcp4821. +config MCP48FEB02 + tristate "MCP48FxBy1/2/4/8 driver" + depends on SPI + help + Say yes here if you want to build the driver for the Microchip: + - 8-bit DAC: + MCP48FEB01, MCP48FEB02, MCP48FEB04, MCP48FEB08, + MCP48FVB01, MCP48FVB02, MCP48FVB04, MCP48FVB08 + - 10-bit DAC: + MCP48FEB11, MCP48FEB12, MCP48FEB14, MCP48FEB18, + MCP48FVB11, MCP48FVB12, MCP48FVB14, MCP48FVB18 + - 12-bit DAC: + MCP48FEB21, MCP48FEB22, MCP48FEB24, MCP48FEB28, + MCP48FVB21, MCP48FVB22, MCP48FVB24, MCP48FVB28 + having 1 to 8 channels, 8/10/12-bit digital-to-analog converter + (DAC) with SPI interface. + + To compile this driver as a module, choose M here: the module + will be called mcp48feb02. + config MCP4922 tristate "MCP4902, MCP4912, MCP4922 DAC driver" depends on SPI diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile index 3684cd52b7fa9bc0ad9f855323dcbb2e4965c404..77b473823bdcc265797169a2bcc263b110092faa 100644 --- a/drivers/iio/dac/Makefile +++ b/drivers/iio/dac/Makefile @@ -51,6 +51,7 @@ obj-$(CONFIG_MAX5821) += max5821.o obj-$(CONFIG_MCP4725) += mcp4725.o obj-$(CONFIG_MCP4728) += mcp4728.o obj-$(CONFIG_MCP4821) += mcp4821.o +obj-$(CONFIG_MCP48FEB02) += mcp48feb02.o obj-$(CONFIG_MCP4922) += mcp4922.o obj-$(CONFIG_STM32_DAC_CORE) += stm32-dac-core.o obj-$(CONFIG_STM32_DAC) += stm32-dac.o diff --git a/drivers/iio/dac/mcp48feb02.c b/drivers/iio/dac/mcp48feb02.c new file mode 100644 index 0000000000000000000000000000000000000000..20955f77053329a9c385f55c7314032eb6b04dfd --- /dev/null +++ b/drivers/iio/dac/mcp48feb02.c @@ -0,0 +1,1243 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * IIO driver for MCP48FEB02 Multi-Channel DAC with SPI interface + * + * Copyright (C) 2026 Microchip Technology Inc. and its subsidiaries + * + * Author: Ariana Lazar <ariana.lazar@microchip.com> + * + * Datasheet links: + * [MCP48FEBxx] https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005429B.pdf + * [MCP48FVBxx] https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005466A.pdf + * [MCP48FxBx4/8] https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/MCP48FXBX4-8-Family-Data-Sheet-DS20006362A.pdf + */ +#include <linux/array_size.h> +#include <linux/bits.h> +#include <linux/bitfield.h> +#include <linux/cleanup.h> +#include <linux/delay.h> +#include <linux/err.h> +#include <linux/iio/iio.h> +#include <linux/iio/sysfs.h> +#include <linux/kstrtox.h> +#include <linux/module.h> +#include <linux/mod_devicetable.h> +#include <linux/mutex.h> +#include <linux/property.h> +#include <linux/spi/spi.h> +#include <linux/time64.h> +#include <linux/types.h> +#include <linux/regmap.h> +#include <linux/regulator/consumer.h> +#include <linux/units.h> + +/* Register addresses must be left shifted with 3 positions in order to append command mask */ +#define MCP48FEB02_DAC0_REG_ADDR 0x00 +#define MCP48FEB02_VREF_REG_ADDR 0x40 +#define MCP48FEB02_POWER_DOWN_REG_ADDR 0x48 +#define MCP48FEB02_DAC_CTRL_MASK GENMASK(1, 0) + +#define MCP48FEB02_GAIN_CTRL_STATUS_REG_ADDR 0x50 +#define MCP48FEB02_GAIN_BIT_MASK BIT(0) +#define MCP48FEB02_GAIN_BIT_STATUS_EEWA_MASK BIT(6) +#define MCP48FEB02_GAIN_BITS_MASK GENMASK(15, 8) + +#define MCP48FEB02_WIPERLOCK_STATUS_REG_ADDR 0x58 + +#define MCP48FEB02_NV_DAC0_REG_ADDR 0x80 +#define MCP48FEB02_NV_VREF_REG_ADDR 0xC0 +#define MCP48FEB02_NV_POWER_DOWN_REG_ADDR 0xC8 +#define MCP48FEB02_NV_GAIN_CTRL_I2C_SLAVE_REG_ADDR 0xD0 +#define MCP48FEB02_NV_I2C_SLAVE_ADDR_MASK GENMASK(7, 0) + +/* Voltage reference, Power-Down control register and DAC Wiperlock status register fields */ +#define DAC_CTRL_MASK(ch) (GENMASK(1, 0) << (2 * (ch))) +#define DAC_CTRL_VAL(ch, val) ((val) << (2 * (ch))) + +/* Gain Control and I2C Slave Address Reguster fields */ +#define DAC_GAIN_MASK(ch) (BIT(0) << (8 + (ch))) +#define DAC_GAIN_VAL(ch, val) ((val) << (8 + (ch))) + +#define REG_ADDR(reg) ((reg) << 3) +#define NV_REG_ADDR(reg) ((NV_DAC_ADDR_OFFSET + (reg)) << 3) +#define READFLAG_MASK GENMASK(2, 1) + +#define MCP48FEB02_MAX_CH 8 +#define MCP48FEB02_MAX_SCALES_CH 3 +#define MCP48FEB02_DAC_WIPER_UNLOCKED 0 +#define MCP48FEB02_NORMAL_OPERATION 0 +#define MCP48FEB02_INTERNAL_BAND_GAP_uV 2440000 +#define NV_DAC_ADDR_OFFSET 0x10 + +enum mcp48feb02_vref_mode { + MCP48FEB02_VREF_VDD = 0, + MCP48FEB02_INTERNAL_BAND_GAP = 1, + MCP48FEB02_EXTERNAL_VREF_UNBUFFERED = 2, + MCP48FEB02_EXTERNAL_VREF_BUFFERED = 3, +}; + +enum mcp48feb02_scale { + MCP48FEB02_SCALE_VDD = 0, + MCP48FEB02_SCALE_GAIN_X1 = 1, + MCP48FEB02_SCALE_GAIN_X2 = 2, +}; + +enum mcp48feb02_gain_bit_mode { + MCP48FEB02_GAIN_BIT_X1 = 0, + MCP48FEB02_GAIN_BIT_X2 = 1, +}; + +static const char * const mcp48feb02_powerdown_modes[] = { + "1kohm_to_gnd", + "100kohm_to_gnd", + "open_circuit", +}; + +/** + * struct mcp48feb02_features - chip specific data + * @name: device name + * @phys_channels: number of hardware channels + * @resolution: DAC resolution + * @have_ext_vref1: does the hardware have an the second external voltage reference? + * @have_eeprom: does the hardware have an internal eeprom? + */ +struct mcp48feb02_features { + const char *name; + unsigned int phys_channels; + unsigned int resolution; + bool have_ext_vref1; + bool have_eeprom; +}; + +static const struct mcp48feb02_features mcp48feb01_chip_features = { + .name = "mcp48feb01", + .phys_channels = 1, + .resolution = 8, + .have_ext_vref1 = false, + .have_eeprom = true, +}; + +static const struct mcp48feb02_features mcp48feb02_chip_features = { + .name = "mcp48feb02", + .phys_channels = 2, + .resolution = 8, + .have_ext_vref1 = false, + .have_eeprom = true, +}; + +static const struct mcp48feb02_features mcp48feb04_chip_features = { + .name = "mcp48feb04", + .phys_channels = 4, + .resolution = 8, + .have_ext_vref1 = true, + .have_eeprom = true, +}; + +static const struct mcp48feb02_features mcp48feb08_chip_features = { + .name = "mcp48feb08", + .phys_channels = 8, + .resolution = 8, + .have_ext_vref1 = true, + .have_eeprom = true, +}; + +static const struct mcp48feb02_features mcp48feb11_chip_features = { + .name = "mcp48feb11", + .phys_channels = 1, + .resolution = 10, + .have_ext_vref1 = false, + .have_eeprom = true, +}; + +static const struct mcp48feb02_features mcp48feb12_chip_features = { + .name = "mcp48feb12", + .phys_channels = 2, + .resolution = 10, + .have_ext_vref1 = false, + .have_eeprom = true, +}; + +static const struct mcp48feb02_features mcp48feb14_chip_features = { + .name = "mcp48feb14", + .phys_channels = 4, + .resolution = 10, + .have_ext_vref1 = true, + .have_eeprom = true, +}; + +static const struct mcp48feb02_features mcp48feb18_chip_features = { + .name = "mcp48feb18", + .phys_channels = 8, + .resolution = 10, + .have_ext_vref1 = true, + .have_eeprom = true, +}; + +static const struct mcp48feb02_features mcp48feb21_chip_features = { + .name = "mcp48feb21", + .phys_channels = 1, + .resolution = 12, + .have_ext_vref1 = false, + .have_eeprom = true, +}; + +static const struct mcp48feb02_features mcp48feb22_chip_features = { + .name = "mcp48feb22", + .phys_channels = 2, + .resolution = 12, + .have_ext_vref1 = false, + .have_eeprom = true, +}; + +static const struct mcp48feb02_features mcp48feb24_chip_features = { + .name = "mcp48feb24", + .phys_channels = 4, + .resolution = 12, + .have_ext_vref1 = true, + .have_eeprom = true, +}; + +static const struct mcp48feb02_features mcp48feb28_chip_features = { + .name = "mcp48feb28", + .phys_channels = 8, + .resolution = 12, + .have_ext_vref1 = true, + .have_eeprom = true, +}; + +static const struct mcp48feb02_features mcp48fvb01_chip_features = { + .name = "mcp48fvb01", + .phys_channels = 1, + .resolution = 8, + .have_ext_vref1 = false, + .have_eeprom = false, +}; + +static const struct mcp48feb02_features mcp48fvb02_chip_features = { + .name = "mcp48fvb02", + .phys_channels = 2, + .resolution = 8, + .have_ext_vref1 = false, + .have_eeprom = false, +}; + +static const struct mcp48feb02_features mcp48fvb04_chip_features = { + .name = "mcp48fvb04", + .phys_channels = 4, + .resolution = 8, + .have_ext_vref1 = true, + .have_eeprom = false, +}; + +static const struct mcp48feb02_features mcp48fvb08_chip_features = { + .name = "mcp48fvb08", + .phys_channels = 8, + .resolution = 8, + .have_ext_vref1 = true, + .have_eeprom = false, +}; + +static const struct mcp48feb02_features mcp48fvb11_chip_features = { + .name = "mcp48fvb11", + .phys_channels = 1, + .resolution = 10, + .have_ext_vref1 = false, + .have_eeprom = false, +}; + +static const struct mcp48feb02_features mcp48fvb12_chip_features = { + .name = "mcp48fvb12", + .phys_channels = 2, + .resolution = 10, + .have_ext_vref1 = false, + .have_eeprom = false, +}; + +static const struct mcp48feb02_features mcp48fvb14_chip_features = { + .name = "mcp48fvb14", + .phys_channels = 4, + .resolution = 10, + .have_ext_vref1 = true, + .have_eeprom = false, +}; + +static const struct mcp48feb02_features mcp48fvb18_chip_features = { + .name = "mcp48fvb18", + .phys_channels = 8, + .resolution = 10, + .have_ext_vref1 = true, + .have_eeprom = false, +}; + +static const struct mcp48feb02_features mcp48fvb21_chip_features = { + .name = "mcp48fvb21", + .phys_channels = 1, + .resolution = 12, + .have_ext_vref1 = false, + .have_eeprom = false, +}; + +static const struct mcp48feb02_features mcp48fvb22_chip_features = { + .name = "mcp48fvb22", + .phys_channels = 2, + .resolution = 12, + .have_ext_vref1 = false, + .have_eeprom = false, +}; + +static const struct mcp48feb02_features mcp48fvb24_chip_features = { + .name = "mcp48fvb24", + .phys_channels = 4, + .resolution = 12, + .have_ext_vref1 = true, + .have_eeprom = false, +}; + +static const struct mcp48feb02_features mcp48fvb28_chip_features = { + .name = "mcp48fvb28", + .phys_channels = 8, + .resolution = 12, + .have_ext_vref1 = true, + .have_eeprom = false, +}; + +/** + * struct mcp48feb02_channel_data - channel configuration + * @ref_mode: chosen voltage for reference + * @use_2x_gain: output driver gain control + * @powerdown: is false if the channel is in normal operation mode + * @powerdown_mode: selected power-down mode + * @dac_data: dac value + */ +struct mcp48feb02_channel_data { + u8 ref_mode; + bool use_2x_gain; + bool powerdown; + u8 powerdown_mode; + u16 dac_data; +}; + +/** + * struct mcp48feb02_data - chip configuration + * @chdata: options configured for each channel on the device + * @lock: prevents concurrent reads/writes to driver's state members + * @chip_features: pointer to features struct + * @scale_1: scales set on channels that are based on Vref1 + * @scale: scales set on channels that are based on Vref/Vref0 + * @active_channels_mask: enabled channels + * @regmap: regmap for directly accessing device register + * @labels: table with channels labels + * @phys_channels: physical channels on the device + * @vref1_buffered: Vref1 buffer is enabled + * @vref_buffered: Vref/Vref0 buffer is enabled + * @use_vref1: vref1-supply is defined + * @use_vref: vref-supply is defined + */ +struct mcp48feb02_data { + struct mcp48feb02_channel_data chdata[MCP48FEB02_MAX_CH]; + struct mutex lock; /* prevents concurrent reads/writes to driver's state members */ + const struct mcp48feb02_features *chip_features; + int scale_1[2 * MCP48FEB02_MAX_SCALES_CH]; + int scale[2 * MCP48FEB02_MAX_SCALES_CH]; + unsigned long active_channels_mask; + struct regmap *regmap; + const char *labels[MCP48FEB02_MAX_CH]; + u16 phys_channels; + bool vref1_buffered; + bool vref_buffered; + bool use_vref1; + bool use_vref; +}; + +static const struct regmap_range mcp48feb02_readable_ranges[] = { + regmap_reg_range(MCP48FEB02_DAC0_REG_ADDR, MCP48FEB02_WIPERLOCK_STATUS_REG_ADDR), + regmap_reg_range(MCP48FEB02_NV_DAC0_REG_ADDR, MCP48FEB02_NV_GAIN_CTRL_I2C_SLAVE_REG_ADDR), +}; + +static const struct regmap_range mcp48feb02_writable_ranges[] = { + regmap_reg_range(MCP48FEB02_DAC0_REG_ADDR, MCP48FEB02_WIPERLOCK_STATUS_REG_ADDR), + regmap_reg_range(MCP48FEB02_NV_DAC0_REG_ADDR, MCP48FEB02_NV_GAIN_CTRL_I2C_SLAVE_REG_ADDR), +}; + +static const struct regmap_range mcp48feb02_volatile_ranges[] = { + regmap_reg_range(MCP48FEB02_DAC0_REG_ADDR, MCP48FEB02_WIPERLOCK_STATUS_REG_ADDR), + regmap_reg_range(MCP48FEB02_NV_DAC0_REG_ADDR, MCP48FEB02_NV_GAIN_CTRL_I2C_SLAVE_REG_ADDR), + regmap_reg_range(MCP48FEB02_DAC0_REG_ADDR, MCP48FEB02_WIPERLOCK_STATUS_REG_ADDR), + regmap_reg_range(MCP48FEB02_NV_DAC0_REG_ADDR, MCP48FEB02_NV_GAIN_CTRL_I2C_SLAVE_REG_ADDR), +}; + +static const struct regmap_access_table mcp48feb02_readable_table = { + .yes_ranges = mcp48feb02_readable_ranges, + .n_yes_ranges = ARRAY_SIZE(mcp48feb02_readable_ranges), +}; + +static const struct regmap_access_table mcp48feb02_writable_table = { + .yes_ranges = mcp48feb02_writable_ranges, + .n_yes_ranges = ARRAY_SIZE(mcp48feb02_writable_ranges), +}; + +static const struct regmap_access_table mcp48feb02_volatile_table = { + .yes_ranges = mcp48feb02_volatile_ranges, + .n_yes_ranges = ARRAY_SIZE(mcp48feb02_volatile_ranges), +}; + +static const struct regmap_config mcp48feb02_regmap_config = { + .name = "mcp48feb02_regmap", + .reg_bits = 8, + .val_bits = 16, + .rd_table = &mcp48feb02_readable_table, + .wr_table = &mcp48feb02_writable_table, + .volatile_table = &mcp48feb02_volatile_table, + .max_register = MCP48FEB02_NV_GAIN_CTRL_I2C_SLAVE_REG_ADDR, + .read_flag_mask = READFLAG_MASK, + .cache_type = REGCACHE_MAPLE, + .val_format_endian = REGMAP_ENDIAN_BIG, +}; + +/* For devices that doesn't have nonvolatile memory */ +static const struct regmap_range mcp48fvb02_readable_ranges[] = { + regmap_reg_range(MCP48FEB02_DAC0_REG_ADDR, MCP48FEB02_WIPERLOCK_STATUS_REG_ADDR), +}; + +static const struct regmap_range mcp48fvb02_writable_ranges[] = { + regmap_reg_range(MCP48FEB02_DAC0_REG_ADDR, MCP48FEB02_WIPERLOCK_STATUS_REG_ADDR), +}; + +static const struct regmap_range mcp48fvb02_volatile_ranges[] = { + regmap_reg_range(MCP48FEB02_DAC0_REG_ADDR, MCP48FEB02_WIPERLOCK_STATUS_REG_ADDR), + regmap_reg_range(MCP48FEB02_DAC0_REG_ADDR, MCP48FEB02_WIPERLOCK_STATUS_REG_ADDR), +}; + +static const struct regmap_access_table mcp48fvb02_readable_table = { + .yes_ranges = mcp48fvb02_readable_ranges, + .n_yes_ranges = ARRAY_SIZE(mcp48fvb02_readable_ranges), +}; + +static const struct regmap_access_table mcp48fvb02_writable_table = { + .yes_ranges = mcp48fvb02_writable_ranges, + .n_yes_ranges = ARRAY_SIZE(mcp48fvb02_writable_ranges), +}; + +static const struct regmap_access_table mcp48fvb02_volatile_table = { + .yes_ranges = mcp48fvb02_volatile_ranges, + .n_yes_ranges = ARRAY_SIZE(mcp48fvb02_volatile_ranges), +}; + +static const struct regmap_config mcp48fvb02_regmap_config = { + .name = "mcp48fvb02_regmap", + .reg_bits = 8, + .val_bits = 16, + .rd_table = &mcp48fvb02_readable_table, + .wr_table = &mcp48fvb02_writable_table, + .volatile_table = &mcp48fvb02_volatile_table, + .max_register = MCP48FEB02_WIPERLOCK_STATUS_REG_ADDR, + .read_flag_mask = READFLAG_MASK, + .cache_type = REGCACHE_MAPLE, + .val_format_endian = REGMAP_ENDIAN_BIG, +}; + +static int mcp48feb02_write_to_eeprom(struct mcp48feb02_data *data, unsigned int reg, + unsigned int val) +{ + int eewa_val, ret; + + ret = regmap_read_poll_timeout(data->regmap, MCP48FEB02_GAIN_CTRL_STATUS_REG_ADDR, + eewa_val, + !(eewa_val & MCP48FEB02_GAIN_BIT_STATUS_EEWA_MASK), + USEC_PER_MSEC, USEC_PER_MSEC * 5); + if (ret) + return ret; + + return regmap_write(data->regmap, reg, val); +} + +static ssize_t store_eeprom_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t len) +{ + struct mcp48feb02_data *data = iio_priv(dev_to_iio_dev(dev)); + unsigned int i, val, val1, eewa_val; + bool state; + int ret; + + ret = kstrtobool(buf, &state); + if (ret) + return ret; + + if (!state) + return 0; + + /* + * Wait until the currently occurring EEPROM Write Cycle is completed. + * Only serial commands to the volatile memory are allowed. + */ + guard(mutex)(&data->lock); + + /* + * Verify DAC Wiper and DAC Configuration are unlocked. If both are disabled, + * writing to EEPROM is available. + */ + ret = regmap_read(data->regmap, MCP48FEB02_WIPERLOCK_STATUS_REG_ADDR, &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 = mcp48feb02_write_to_eeprom(data, NV_REG_ADDR(i), + data->chdata[i].dac_data); + if (ret) + return ret; + } + + ret = regmap_read(data->regmap, MCP48FEB02_VREF_REG_ADDR, &val); + if (ret) + return ret; + + ret = mcp48feb02_write_to_eeprom(data, MCP48FEB02_NV_VREF_REG_ADDR, val); + if (ret) + return ret; + + ret = regmap_read(data->regmap, MCP48FEB02_POWER_DOWN_REG_ADDR, &val); + if (ret) + return ret; + + ret = mcp48feb02_write_to_eeprom(data, MCP48FEB02_NV_POWER_DOWN_REG_ADDR, val); + if (ret) + return ret; + + ret = regmap_read_poll_timeout(data->regmap, MCP48FEB02_GAIN_CTRL_STATUS_REG_ADDR, eewa_val, + !(eewa_val & MCP48FEB02_GAIN_BIT_STATUS_EEWA_MASK), + USEC_PER_MSEC, USEC_PER_MSEC * 5); + if (ret) + return ret; + + ret = regmap_read(data->regmap, MCP48FEB02_NV_GAIN_CTRL_I2C_SLAVE_REG_ADDR, &val); + if (ret) + return ret; + + ret = regmap_read(data->regmap, MCP48FEB02_GAIN_CTRL_STATUS_REG_ADDR, &val1); + if (ret) + return ret; + + ret = mcp48feb02_write_to_eeprom(data, MCP48FEB02_NV_GAIN_CTRL_I2C_SLAVE_REG_ADDR, + (val1 & MCP48FEB02_GAIN_BITS_MASK) | + (val & MCP48FEB02_NV_I2C_SLAVE_ADDR_MASK)); + if (ret) + return ret; + + return len; +} + +static IIO_DEVICE_ATTR_WO(store_eeprom, 0); + +static struct attribute *mcp48feb02_attributes[] = { + &iio_dev_attr_store_eeprom.dev_attr.attr, + NULL +}; + +static const struct attribute_group mcp48feb02_attribute_group = { + .attrs = mcp48feb02_attributes, +}; + +static int mcp48feb02_suspend(struct device *dev) +{ + struct iio_dev *indio_dev = dev_get_drvdata(dev); + struct mcp48feb02_data *data = iio_priv(indio_dev); + int ret; + u8 ch; + + guard(mutex)(&data->lock); + + for_each_set_bit(ch, &data->active_channels_mask, data->phys_channels) { + u8 pd_mode; + + data->chdata[ch].powerdown = true; + pd_mode = data->chdata[ch].powerdown_mode + 1; + ret = regmap_update_bits(data->regmap, MCP48FEB02_POWER_DOWN_REG_ADDR, + DAC_CTRL_MASK(ch), DAC_CTRL_VAL(ch, pd_mode)); + if (ret) + return ret; + + ret = regmap_write(data->regmap, REG_ADDR(ch), data->chdata[ch].dac_data); + if (ret) + return ret; + } + + return 0; +} + +static int mcp48feb02_resume(struct device *dev) +{ + struct iio_dev *indio_dev = dev_get_drvdata(dev); + struct mcp48feb02_data *data = iio_priv(indio_dev); + u8 ch; + + guard(mutex)(&data->lock); + + for_each_set_bit(ch, &data->active_channels_mask, data->phys_channels) { + int ret; + + data->chdata[ch].powerdown = false; + + ret = regmap_write(data->regmap, REG_ADDR(ch), data->chdata[ch].dac_data); + if (ret) + return ret; + + ret = regmap_update_bits(data->regmap, MCP48FEB02_VREF_REG_ADDR, + DAC_CTRL_MASK(ch), + DAC_CTRL_VAL(ch, data->chdata[ch].ref_mode)); + if (ret) + return ret; + + ret = regmap_update_bits(data->regmap, MCP48FEB02_GAIN_CTRL_STATUS_REG_ADDR, + DAC_GAIN_MASK(ch), + DAC_GAIN_VAL(ch, data->chdata[ch].use_2x_gain)); + if (ret) + return ret; + + ret = regmap_update_bits(data->regmap, MCP48FEB02_POWER_DOWN_REG_ADDR, + DAC_CTRL_MASK(ch), + DAC_CTRL_VAL(ch, MCP48FEB02_NORMAL_OPERATION)); + if (ret) + return ret; + } + + return 0; +} + +static int mcp48feb02_get_powerdown_mode(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan) +{ + struct mcp48feb02_data *data = iio_priv(indio_dev); + + return data->chdata[chan->address].powerdown_mode; +} + +static int mcp48feb02_set_powerdown_mode(struct iio_dev *indio_dev, const struct iio_chan_spec *ch, + unsigned int mode) +{ + struct mcp48feb02_data *data = iio_priv(indio_dev); + + data->chdata[ch->address].powerdown_mode = mode; + + return 0; +} + +static ssize_t mcp48feb02_read_powerdown(struct iio_dev *indio_dev, uintptr_t private, + const struct iio_chan_spec *ch, char *buf) +{ + struct mcp48feb02_data *data = iio_priv(indio_dev); + + /* Print if channel is in a power-down mode or not */ + return sysfs_emit(buf, "%d\n", data->chdata[ch->address].powerdown); +} + +static ssize_t mcp48feb02_write_powerdown(struct iio_dev *indio_dev, uintptr_t private, + const struct iio_chan_spec *ch, const char *buf, + size_t len) +{ + struct mcp48feb02_data *data = iio_priv(indio_dev); + u32 reg = ch->address; + u8 tmp_pd_mode; + bool state; + int ret; + + guard(mutex)(&data->lock); + + ret = kstrtobool(buf, &state); + if (ret) + return ret; + + /* + * Set the channel to the specified power-down mode. Exiting power-down mode + * requires writing normal operation mode (0) to the channel-specific register bits. + */ + tmp_pd_mode = state ? (data->chdata[reg].powerdown_mode + 1) : MCP48FEB02_NORMAL_OPERATION; + ret = regmap_update_bits(data->regmap, MCP48FEB02_POWER_DOWN_REG_ADDR, + DAC_CTRL_MASK(reg), DAC_CTRL_VAL(reg, tmp_pd_mode)); + if (ret) + return ret; + + data->chdata[reg].powerdown = state; + + return len; +} + +static DEFINE_SIMPLE_DEV_PM_OPS(mcp48feb02_pm_ops, mcp48feb02_suspend, mcp48feb02_resume); + +static const struct iio_enum mcp48febxx_powerdown_mode_enum = { + .items = mcp48feb02_powerdown_modes, + .num_items = ARRAY_SIZE(mcp48feb02_powerdown_modes), + .get = mcp48feb02_get_powerdown_mode, + .set = mcp48feb02_set_powerdown_mode, +}; + +static const struct iio_chan_spec_ext_info mcp48feb02_ext_info[] = { + { + .name = "powerdown", + .read = mcp48feb02_read_powerdown, + .write = mcp48feb02_write_powerdown, + .shared = IIO_SEPARATE, + }, + IIO_ENUM("powerdown_mode", IIO_SEPARATE, &mcp48febxx_powerdown_mode_enum), + IIO_ENUM_AVAILABLE("powerdown_mode", IIO_SHARED_BY_TYPE, &mcp48febxx_powerdown_mode_enum), + { } +}; + +static const struct iio_chan_spec mcp48febxx_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 = mcp48feb02_ext_info, +}; + +static void mcp48feb02_init_scale(struct mcp48feb02_data *data, enum mcp48feb02_scale scale, + int vref_uV, int scale_avail[]) +{ + u32 value_micro, value_int; + u64 tmp; + + /* vref_uV should not be negative */ + tmp = (u64)vref_uV * MILLI >> data->chip_features->resolution; + value_int = div_u64_rem(tmp, MICRO, &value_micro); + scale_avail[scale * 2] = value_int; + scale_avail[scale * 2 + 1] = value_micro; +} + +static int mcp48feb02_init_scales_avail(struct mcp48feb02_data *data, int vdd_uV, + int vref_uV, int vref1_uV) +{ + int tmp_vref; + + mcp48feb02_init_scale(data, MCP48FEB02_SCALE_VDD, vdd_uV, data->scale); + + tmp_vref = data->use_vref ? vref_uV : MCP48FEB02_INTERNAL_BAND_GAP_uV; + mcp48feb02_init_scale(data, MCP48FEB02_SCALE_GAIN_X1, tmp_vref, data->scale); + mcp48feb02_init_scale(data, MCP48FEB02_SCALE_GAIN_X2, tmp_vref * 2, data->scale); + + if (data->phys_channels >= 4) { + mcp48feb02_init_scale(data, MCP48FEB02_SCALE_VDD, vdd_uV, data->scale_1); + + if (data->use_vref1) + tmp_vref = vref1_uV; + else + tmp_vref = MCP48FEB02_INTERNAL_BAND_GAP_uV; + + mcp48feb02_init_scale(data, MCP48FEB02_SCALE_GAIN_X1, + tmp_vref, data->scale_1); + mcp48feb02_init_scale(data, MCP48FEB02_SCALE_GAIN_X2, + tmp_vref * 2, data->scale_1); + } + + return 0; +} + +static int mcp48feb02_read_avail(struct iio_dev *indio_dev, struct iio_chan_spec const *ch, + const int **vals, int *type, int *length, long info) +{ + struct mcp48feb02_data *data = iio_priv(indio_dev); + + switch (info) { + case IIO_CHAN_INFO_SCALE: + switch (ch->type) { + case IIO_VOLTAGE: + if (data->phys_channels >= 4 && (ch->address % 2)) + *vals = data->scale_1; + else + *vals = data->scale; + + *length = 2 * MCP48FEB02_MAX_SCALES_CH; + *type = IIO_VAL_INT_PLUS_MICRO; + return IIO_AVAIL_LIST; + default: + return -EINVAL; + } + default: + return -EINVAL; + } +} + +static void mcp48feb02_get_scale(int ch, struct mcp48feb02_data *data, int *val, int *val2) +{ + enum mcp48feb02_scale current_scale; + + if (data->chdata[ch].ref_mode == MCP48FEB02_VREF_VDD) + current_scale = MCP48FEB02_SCALE_VDD; + else if (data->chdata[ch].use_2x_gain) + current_scale = MCP48FEB02_SCALE_GAIN_X2; + else + current_scale = MCP48FEB02_SCALE_GAIN_X1; + + if (data->phys_channels >= 4 && (ch % 2)) { + *val = data->scale_1[current_scale * 2]; + *val2 = data->scale_1[current_scale * 2 + 1]; + } else { + *val = data->scale[current_scale * 2]; + *val2 = data->scale[current_scale * 2 + 1]; + } +} + +static int mcp48feb02_check_scale(struct mcp48feb02_data *data, int val, int val2, int scale[]) +{ + unsigned int i; + + for (i = 0; i < MCP48FEB02_MAX_SCALES_CH; i++) { + if (scale[i * 2] == val && scale[i * 2 + 1] == val2) + return i; + } + + return -EINVAL; +} + +static int mcp48feb02_ch_scale(struct mcp48feb02_data *data, int ch, int scale) +{ + int tmp_val, ret; + + if (scale == MCP48FEB02_SCALE_VDD) { + tmp_val = MCP48FEB02_VREF_VDD; + } else if (data->phys_channels >= 4 && (ch % 2)) { + if (data->use_vref1) { + if (data->vref1_buffered) + tmp_val = MCP48FEB02_EXTERNAL_VREF_BUFFERED; + else + tmp_val = MCP48FEB02_EXTERNAL_VREF_UNBUFFERED; + } else { + tmp_val = MCP48FEB02_INTERNAL_BAND_GAP; + } + } else if (data->use_vref) { + if (data->vref_buffered) + tmp_val = MCP48FEB02_EXTERNAL_VREF_BUFFERED; + else + tmp_val = MCP48FEB02_EXTERNAL_VREF_UNBUFFERED; + } else { + tmp_val = MCP48FEB02_INTERNAL_BAND_GAP; + } + + ret = regmap_update_bits(data->regmap, MCP48FEB02_VREF_REG_ADDR, + DAC_CTRL_MASK(ch), DAC_CTRL_VAL(ch, tmp_val)); + if (ret) + return ret; + + data->chdata[ch].ref_mode = tmp_val; + + return 0; +} + +/* + * Setting the scale in order to choose between VDD and (Vref or Band Gap) from the user + * space. The VREF pin is either an input or an output, therefore the user cannot + * simultaneously connect an external voltage reference to the pin and select the + * internal Band Gap. + * When the DAC’s voltage reference is configured as the VREF pin, the pin is an input. + * When the DAC’s voltage reference is configured as the internal Band Gap, + * the VREF pin is an output. + * If Vref/Vref1 voltage is not available, then the internal Band Gap will be used + * to calculate the values for the scale. + */ +static int mcp48feb02_set_scale(struct mcp48feb02_data *data, int ch, int scale) +{ + int tmp_val, ret; + + ret = mcp48feb02_ch_scale(data, ch, scale); + if (ret) + return ret; + + if (scale == MCP48FEB02_SCALE_GAIN_X2) + tmp_val = MCP48FEB02_GAIN_BIT_X2; + else + tmp_val = MCP48FEB02_GAIN_BIT_X1; + + ret = regmap_update_bits(data->regmap, MCP48FEB02_GAIN_CTRL_STATUS_REG_ADDR, + DAC_GAIN_MASK(ch), DAC_GAIN_VAL(ch, tmp_val)); + if (ret) + return ret; + + data->chdata[ch].use_2x_gain = tmp_val; + + return 0; +} + +static int mcp48feb02_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *ch, + int *val, int *val2, long mask) +{ + struct mcp48feb02_data *data = iio_priv(indio_dev); + int ret; + + switch (mask) { + case IIO_CHAN_INFO_RAW: + ret = regmap_read(data->regmap, REG_ADDR(ch->address), val); + if (ret) + return ret; + return IIO_VAL_INT; + case IIO_CHAN_INFO_SCALE: + mcp48feb02_get_scale(ch->address, data, val, val2); + return IIO_VAL_INT_PLUS_MICRO; + default: + return -EINVAL; + } +} + +static int mcp48feb02_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *ch, + int val, int val2, long mask) +{ + struct mcp48feb02_data *data = iio_priv(indio_dev); + int *tmp_scale, ret; + + guard(mutex)(&data->lock); + + switch (mask) { + case IIO_CHAN_INFO_RAW: + ret = regmap_write(data->regmap, REG_ADDR(ch->address), val); + if (ret) + return ret; + + data->chdata[ch->address].dac_data = val; + return 0; + case IIO_CHAN_INFO_SCALE: + if (data->phys_channels >= 4 && (ch->address % 2)) + tmp_scale = data->scale_1; + else + tmp_scale = data->scale; + + ret = mcp48feb02_check_scale(data, val, val2, tmp_scale); + if (ret < 0) + return ret; + + return mcp48feb02_set_scale(data, ch->address, ret); + default: + return -EINVAL; + } +} + +static int mcp48feb02_read_label(struct iio_dev *indio_dev, struct iio_chan_spec const *ch, + char *label) +{ + struct mcp48feb02_data *data = iio_priv(indio_dev); + + return sysfs_emit(label, "%s\n", data->labels[ch->address]); +} + +static const struct iio_info mcp48feb02_info = { + .read_raw = mcp48feb02_read_raw, + .write_raw = mcp48feb02_write_raw, + .read_label = mcp48feb02_read_label, + .read_avail = &mcp48feb02_read_avail, + .attrs = &mcp48feb02_attribute_group, +}; + +static const struct iio_info mcp48fvb02_info = { + .read_raw = mcp48feb02_read_raw, + .write_raw = mcp48feb02_write_raw, + .read_label = mcp48feb02_read_label, + .read_avail = &mcp48feb02_read_avail, +}; + +static int mcp48feb02_parse_fw(struct iio_dev *indio_dev, + const struct mcp48feb02_features *chip_features) +{ + struct iio_chan_spec chanspec = mcp48febxx_ch_template; + struct mcp48feb02_data *data = iio_priv(indio_dev); + struct device *dev = regmap_get_device(data->regmap); + struct iio_chan_spec *channels; + u32 num_channels; + u8 chan_idx = 0; + + guard(mutex)(&data->lock); + + num_channels = device_get_child_node_count(dev); + if (num_channels > chip_features->phys_channels) + return dev_err_probe(dev, -EINVAL, "More channels than the chip supports\n"); + + if (!num_channels) + return dev_err_probe(dev, -EINVAL, "No channel specified in the devicetree.\n"); + + channels = devm_kcalloc(dev, num_channels, sizeof(*channels), GFP_KERNEL); + if (!channels) + return -ENOMEM; + + device_for_each_child_node_scoped(dev, child) { + u32 reg = 0; + int ret; + + ret = fwnode_property_read_u32(child, "reg", ®); + if (ret) + return dev_err_probe(dev, ret, "Invalid channel number\n"); + + if (reg >= chip_features->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); + + ret = fwnode_property_read_string(child, "label", &data->labels[reg]); + if (ret) + return dev_err_probe(dev, ret, "%pfw: invalid label\n", + fwnode_get_name(child)); + + 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 = chip_features->phys_channels; + + data->vref_buffered = device_property_read_bool(dev, "microchip,vref-buffered"); + + if (chip_features->have_ext_vref1) + data->vref1_buffered = device_property_read_bool(dev, "microchip,vref1-buffered"); + + return 0; +} + +static int mcp48feb02_init_ctrl_regs(struct mcp48feb02_data *data) +{ + unsigned int i, vref_ch, gain_ch, pd_ch; + int ret; + + ret = regmap_read(data->regmap, MCP48FEB02_VREF_REG_ADDR, &vref_ch); + if (ret) + return ret; + + ret = regmap_read(data->regmap, MCP48FEB02_GAIN_CTRL_STATUS_REG_ADDR, &gain_ch); + if (ret) + return ret; + + ret = regmap_read(data->regmap, MCP48FEB02_POWER_DOWN_REG_ADDR, &pd_ch); + if (ret) + return ret; + + gain_ch = gain_ch & MCP48FEB02_GAIN_BITS_MASK; + for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) { + struct device *dev = regmap_get_device(data->regmap); + unsigned int pd_tmp; + + data->chdata[i].ref_mode = (vref_ch >> (2 * i)) & MCP48FEB02_DAC_CTRL_MASK; + data->chdata[i].use_2x_gain = (gain_ch >> i) & MCP48FEB02_GAIN_BIT_MASK; + + /* + * Inform the user that the current voltage reference read from the volatile + * register of the chip is different from the one specified in the device tree. + * Considering that the user cannot have an external voltage reference connected + * to the pin and select the internal Band Gap at the same time, in order to avoid + * miscofiguring the reference voltage, the volatile register will not be written. + * In order to overwrite the setting from volatile register with the one from the + * device tree, the user needs to write the chosen scale. + */ + switch (data->chdata[i].ref_mode) { + case MCP48FEB02_INTERNAL_BAND_GAP: + if (data->phys_channels >= 4 && (i % 2) && data->use_vref1) { + dev_dbg(dev, "ch[%u]: was configured to use internal band gap", i); + dev_dbg(dev, "ch[%u]: reference voltage set to VREF1", i); + break; + } + if ((data->phys_channels < 4 || (data->phys_channels >= 4 && !(i % 2))) && + data->use_vref) { + dev_dbg(dev, "ch[%u]: was configured to use internal band gap", i); + dev_dbg(dev, "ch[%u]: reference voltage set to VREF", i); + break; + } + break; + case MCP48FEB02_EXTERNAL_VREF_UNBUFFERED: + case MCP48FEB02_EXTERNAL_VREF_BUFFERED: + if (data->phys_channels >= 4 && (i % 2) && !data->use_vref1) { + dev_dbg(dev, "ch[%u]: was configured to use VREF1", i); + dev_dbg(dev, + "ch[%u]: reference voltage set to internal band gap", i); + break; + } + if ((data->phys_channels < 4 || (data->phys_channels >= 4 && !(i % 2))) && + !data->use_vref) { + dev_dbg(dev, "ch[%u]: was configured to use VREF", i); + dev_dbg(dev, + "ch[%u]: reference voltage set to internal band gap", i); + break; + } + break; + } + + pd_tmp = (pd_ch >> (2 * i)) & MCP48FEB02_DAC_CTRL_MASK; + data->chdata[i].powerdown_mode = pd_tmp ? (pd_tmp - 1) : pd_tmp; + data->chdata[i].powerdown = !!(data->chdata[i].powerdown_mode); + } + + return 0; +} + +static int mcp48feb02_init_ch_scales(struct mcp48feb02_data *data, int vdd_uV, + int vref_uV, int vref1_uV) +{ + unsigned int i; + + for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) { + struct device *dev = regmap_get_device(data->regmap); + int ret; + + ret = mcp48feb02_init_scales_avail(data, vdd_uV, vref_uV, vref1_uV); + if (ret) + return dev_err_probe(dev, ret, "failed to init scales for ch %u\n", i); + } + + return 0; +} + +static int mcp48feb02_probe(struct spi_device *spi) +{ + const struct mcp48feb02_features *chip_features; + struct device *dev = &spi->dev; + struct mcp48feb02_data *data; + struct iio_dev *indio_dev; + int vref1_uV = 0; + int vref_uV = 0; + int vdd_uV; + int ret; + + indio_dev = devm_iio_device_alloc(dev, sizeof(*data)); + if (!indio_dev) + return -ENOMEM; + + data = iio_priv(indio_dev); + + chip_features = spi_get_device_match_data(spi); + if (!chip_features) + return -EINVAL; + + data->chip_features = chip_features; + + if (chip_features->have_eeprom) { + data->regmap = devm_regmap_init_spi(spi, &mcp48feb02_regmap_config); + indio_dev->info = &mcp48feb02_info; + } else { + data->regmap = devm_regmap_init_spi(spi, &mcp48fvb02_regmap_config); + indio_dev->info = &mcp48fvb02_info; + } + if (IS_ERR(data->regmap)) + return dev_err_probe(dev, PTR_ERR(data->regmap), "Error initializing spi regmap\n"); + + indio_dev->name = chip_features->name; + + ret = mcp48feb02_parse_fw(indio_dev, chip_features); + if (ret) + return dev_err_probe(dev, ret, "Error parsing firmware data\n"); + + ret = devm_mutex_init(dev, &data->lock); + if (ret) + return ret; + + ret = devm_regulator_get_enable_read_voltage(dev, "vdd"); + if (ret < 0) + return ret; + + vdd_uV = ret; + + ret = devm_regulator_get_enable_read_voltage(dev, "vref"); + if (ret > 0) { + vref_uV = ret; + data->use_vref = true; + } else { + dev_dbg(dev, "using internal band gap as voltage reference.\n"); + dev_dbg(dev, "External Vref is unavailable.\n"); + } + + if (chip_features->have_ext_vref1) { + ret = devm_regulator_get_enable_read_voltage(dev, "vref1"); + if (ret > 0) { + vref1_uV = ret; + data->use_vref1 = true; + } else { + dev_dbg(dev, "using internal band gap as voltage reference 1.\n"); + dev_dbg(dev, "External Vref1 is unavailable.\n"); + } + } + + ret = mcp48feb02_init_ctrl_regs(data); + if (ret) + return dev_err_probe(dev, ret, "Error initialising vref register\n"); + + ret = mcp48feb02_init_ch_scales(data, vdd_uV, vref_uV, vref1_uV); + if (ret) + return ret; + + return devm_iio_device_register(dev, indio_dev); +} + +static const struct spi_device_id mcp48feb02_id[] = { + { "mcp48feb01", (kernel_ulong_t)&mcp48feb01_chip_features }, + { "mcp48feb02", (kernel_ulong_t)&mcp48feb02_chip_features }, + { "mcp48feb04", (kernel_ulong_t)&mcp48feb04_chip_features }, + { "mcp48feb08", (kernel_ulong_t)&mcp48feb08_chip_features }, + { "mcp48feb11", (kernel_ulong_t)&mcp48feb11_chip_features }, + { "mcp48feb12", (kernel_ulong_t)&mcp48feb12_chip_features }, + { "mcp48feb14", (kernel_ulong_t)&mcp48feb14_chip_features }, + { "mcp48feb18", (kernel_ulong_t)&mcp48feb18_chip_features }, + { "mcp48feb21", (kernel_ulong_t)&mcp48feb21_chip_features }, + { "mcp48feb22", (kernel_ulong_t)&mcp48feb22_chip_features }, + { "mcp48feb24", (kernel_ulong_t)&mcp48feb24_chip_features }, + { "mcp48feb28", (kernel_ulong_t)&mcp48feb28_chip_features }, + { "mcp48fvb01", (kernel_ulong_t)&mcp48fvb01_chip_features }, + { "mcp48fvb02", (kernel_ulong_t)&mcp48fvb02_chip_features }, + { "mcp48fvb04", (kernel_ulong_t)&mcp48fvb04_chip_features }, + { "mcp48fvb08", (kernel_ulong_t)&mcp48fvb08_chip_features }, + { "mcp48fvb11", (kernel_ulong_t)&mcp48fvb11_chip_features }, + { "mcp48fvb12", (kernel_ulong_t)&mcp48fvb12_chip_features }, + { "mcp48fvb14", (kernel_ulong_t)&mcp48fvb14_chip_features }, + { "mcp48fvb18", (kernel_ulong_t)&mcp48fvb18_chip_features }, + { "mcp48fvb21", (kernel_ulong_t)&mcp48fvb21_chip_features }, + { "mcp48fvb22", (kernel_ulong_t)&mcp48fvb22_chip_features }, + { "mcp48fvb24", (kernel_ulong_t)&mcp48fvb24_chip_features }, + { "mcp48fvb28", (kernel_ulong_t)&mcp48fvb28_chip_features }, + { } +}; +MODULE_DEVICE_TABLE(spi, mcp48feb02_id); + +static const struct of_device_id mcp48feb02_of_match[] = { + { .compatible = "microchip,mcp48feb01", .data = &mcp48feb01_chip_features }, + { .compatible = "microchip,mcp48feb02", .data = &mcp48feb02_chip_features }, + { .compatible = "microchip,mcp48feb04", .data = &mcp48feb04_chip_features }, + { .compatible = "microchip,mcp48feb08", .data = &mcp48feb08_chip_features }, + { .compatible = "microchip,mcp48feb11", .data = &mcp48feb11_chip_features }, + { .compatible = "microchip,mcp48feb12", .data = &mcp48feb12_chip_features }, + { .compatible = "microchip,mcp48feb14", .data = &mcp48feb14_chip_features }, + { .compatible = "microchip,mcp48feb18", .data = &mcp48feb18_chip_features }, + { .compatible = "microchip,mcp48feb21", .data = &mcp48feb21_chip_features }, + { .compatible = "microchip,mcp48feb22", .data = &mcp48feb22_chip_features }, + { .compatible = "microchip,mcp48feb24", .data = &mcp48feb24_chip_features }, + { .compatible = "microchip,mcp48feb28", .data = &mcp48feb28_chip_features }, + { .compatible = "microchip,mcp48fvb01", .data = &mcp48fvb01_chip_features }, + { .compatible = "microchip,mcp48fvb02", .data = &mcp48fvb02_chip_features }, + { .compatible = "microchip,mcp48fvb04", .data = &mcp48fvb04_chip_features }, + { .compatible = "microchip,mcp48fvb08", .data = &mcp48fvb08_chip_features }, + { .compatible = "microchip,mcp48fvb11", .data = &mcp48fvb11_chip_features }, + { .compatible = "microchip,mcp48fvb12", .data = &mcp48fvb12_chip_features }, + { .compatible = "microchip,mcp48fvb14", .data = &mcp48fvb14_chip_features }, + { .compatible = "microchip,mcp48fvb18", .data = &mcp48fvb18_chip_features }, + { .compatible = "microchip,mcp48fvb21", .data = &mcp48fvb21_chip_features }, + { .compatible = "microchip,mcp48fvb22", .data = &mcp48fvb22_chip_features }, + { .compatible = "microchip,mcp48fvb24", .data = &mcp48fvb24_chip_features }, + { .compatible = "microchip,mcp48fvb28", .data = &mcp48fvb28_chip_features }, + { } +}; +MODULE_DEVICE_TABLE(of, mcp48feb02_of_match); + +static struct spi_driver mcp48feb02_driver = { + .driver = { + .name = "mcp48feb02", + .of_match_table = mcp48feb02_of_match, + .pm = pm_sleep_ptr(&mcp48feb02_pm_ops), + }, + .probe = mcp48feb02_probe, + .id_table = mcp48feb02_id, +}; +module_spi_driver(mcp48feb02_driver); + +MODULE_AUTHOR("Ariana Lazar <ariana.lazar@microchip.com>"); +MODULE_DESCRIPTION("IIO driver for MCP48FEB02 Multi-Channel DAC with SPI interface"); +MODULE_LICENSE("GPL"); -- 2.43.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] iio: dac: add support for Microchip MCP48FEB02 2026-02-12 12:48 ` [PATCH 2/2] " Ariana Lazar @ 2026-02-12 14:42 ` Andy Shevchenko 2026-02-16 14:29 ` Ariana.Lazar 2026-02-15 17:58 ` Jonathan Cameron 1 sibling, 1 reply; 18+ messages in thread From: Andy Shevchenko @ 2026-02-12 14:42 UTC (permalink / raw) To: Ariana Lazar Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio, devicetree, linux-kernel On Thu, Feb 12, 2026 at 02:48:35PM +0200, Ariana Lazar wrote: > This is the iio driver for Microchip MCP48FxBy1/2/4/8 series of buffered > voltage output Digital-to-Analog Converters with nonvolatile or volatile > memory and an SPI Interface. > > The families support up to 8 output channels. > > The devices can be 8-bit, 10-bit and 12-bit. ... > +#include <linux/array_size.h> > +#include <linux/bits.h> > +#include <linux/bitfield.h> > +#include <linux/cleanup.h> > +#include <linux/delay.h> > +#include <linux/err.h> > +#include <linux/iio/iio.h> > +#include <linux/iio/sysfs.h> I would split this group... > +#include <linux/kstrtox.h> > +#include <linux/module.h> > +#include <linux/mod_devicetable.h> > +#include <linux/mutex.h> > +#include <linux/property.h> > +#include <linux/spi/spi.h> > +#include <linux/time64.h> > +#include <linux/types.h> > +#include <linux/regmap.h> > +#include <linux/regulator/consumer.h> > +#include <linux/units.h> > + ...to be here as other IIO drivers do. ... > +/* Gain Control and I2C Slave Address Reguster fields */ > +#define DAC_GAIN_MASK(ch) (BIT(0) << (8 + (ch))) Just BIT(8 + (ch)) should suffice. > +#define DAC_GAIN_VAL(ch, val) ((val) << (8 + (ch))) For the sake of consistency this may be also rewritten to #define DAC_GAIN_VAL(ch, val) ((val) * BIT(8 + (ch))) ... > +/** > + * struct mcp48feb02_channel_data - channel configuration > + * @ref_mode: chosen voltage for reference > + * @use_2x_gain: output driver gain control > + * @powerdown: is false if the channel is in normal operation mode > + * @powerdown_mode: selected power-down mode > + * @dac_data: dac value DAC > + */ > +struct mcp48feb02_channel_data { > + u8 ref_mode; > + bool use_2x_gain; > + bool powerdown; > + u8 powerdown_mode; > + u16 dac_data; Wondering if the following arrangement is slightly better: u16 dac_data; u8 ref_mode; u8 powerdown_mode; bool powerdown; bool use_2x_gain; > +}; ... > +/** > + * struct mcp48feb02_data - chip configuration > + * @chdata: options configured for each channel on the device > + * @lock: prevents concurrent reads/writes to driver's state members > + * @chip_features: pointer to features struct > + * @scale_1: scales set on channels that are based on Vref1 > + * @scale: scales set on channels that are based on Vref/Vref0 > + * @active_channels_mask: enabled channels > + * @regmap: regmap for directly accessing device register > + * @labels: table with channels labels > + * @phys_channels: physical channels on the device > + * @vref1_buffered: Vref1 buffer is enabled > + * @vref_buffered: Vref/Vref0 buffer is enabled > + * @use_vref1: vref1-supply is defined > + * @use_vref: vref-supply is defined > + */ > +struct mcp48feb02_data { > + struct mcp48feb02_channel_data chdata[MCP48FEB02_MAX_CH]; > + struct mutex lock; /* prevents concurrent reads/writes to driver's state members */ > + const struct mcp48feb02_features *chip_features; > + int scale_1[2 * MCP48FEB02_MAX_SCALES_CH]; > + int scale[2 * MCP48FEB02_MAX_SCALES_CH]; I would name it scale1 and scale0. This will increase readability to see that the sizes are equal and that the first digit is the part of the name. > + unsigned long active_channels_mask; > + struct regmap *regmap; > + const char *labels[MCP48FEB02_MAX_CH]; > + u16 phys_channels; > + bool vref1_buffered; > + bool vref_buffered; > + bool use_vref1; > + bool use_vref; > +}; ... > +static int mcp48feb02_write_to_eeprom(struct mcp48feb02_data *data, unsigned int reg, > + unsigned int val) > +{ > + int eewa_val, ret; Is it okay that the eewa_val is signed? > + ret = regmap_read_poll_timeout(data->regmap, MCP48FEB02_GAIN_CTRL_STATUS_REG_ADDR, > + eewa_val, > + !(eewa_val & MCP48FEB02_GAIN_BIT_STATUS_EEWA_MASK), > + USEC_PER_MSEC, USEC_PER_MSEC * 5); I would rather put it as 1 * USEC_PER_MSEC, 5 * USEC_PER_MSEC); This follows the natural (from physics) reading — 1ms, 5ms. > + if (ret) > + return ret; > + > + return regmap_write(data->regmap, reg, val); > +} ... > +static ssize_t store_eeprom_store(struct device *dev, struct device_attribute *attr, > + const char *buf, size_t len) > +{ > + struct mcp48feb02_data *data = iio_priv(dev_to_iio_dev(dev)); > + unsigned int i, val, val1, eewa_val; > + bool state; > + int ret; > + > + ret = kstrtobool(buf, &state); > + if (ret) > + return ret; > + > + if (!state) > + return 0; > + > + /* > + * Wait until the currently occurring EEPROM Write Cycle is completed. > + * Only serial commands to the volatile memory are allowed. > + */ > + guard(mutex)(&data->lock); > + > + /* > + * Verify DAC Wiper and DAC Configuration are unlocked. If both are disabled, > + * writing to EEPROM is available. > + */ > + ret = regmap_read(data->regmap, MCP48FEB02_WIPERLOCK_STATUS_REG_ADDR, &val); > + if (ret) > + return ret; > + > + if (val) { > + dev_err(dev, "DAC Wiper and DAC Configuration not are unlocked.\n"); "are not" > + return -EINVAL; > + } > + > + for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) { > + ret = mcp48feb02_write_to_eeprom(data, NV_REG_ADDR(i), > + data->chdata[i].dac_data); > + if (ret) > + return ret; > + } > + > + ret = regmap_read(data->regmap, MCP48FEB02_VREF_REG_ADDR, &val); > + if (ret) > + return ret; > + > + ret = mcp48feb02_write_to_eeprom(data, MCP48FEB02_NV_VREF_REG_ADDR, val); > + if (ret) > + return ret; > + > + ret = regmap_read(data->regmap, MCP48FEB02_POWER_DOWN_REG_ADDR, &val); > + if (ret) > + return ret; > + > + ret = mcp48feb02_write_to_eeprom(data, MCP48FEB02_NV_POWER_DOWN_REG_ADDR, val); > + if (ret) > + return ret; > + > + ret = regmap_read_poll_timeout(data->regmap, MCP48FEB02_GAIN_CTRL_STATUS_REG_ADDR, eewa_val, > + !(eewa_val & MCP48FEB02_GAIN_BIT_STATUS_EEWA_MASK), > + USEC_PER_MSEC, USEC_PER_MSEC * 5); > + if (ret) > + return ret; > + > + ret = regmap_read(data->regmap, MCP48FEB02_NV_GAIN_CTRL_I2C_SLAVE_REG_ADDR, &val); > + if (ret) > + return ret; > + > + ret = regmap_read(data->regmap, MCP48FEB02_GAIN_CTRL_STATUS_REG_ADDR, &val1); > + if (ret) > + return ret; > + > + ret = mcp48feb02_write_to_eeprom(data, MCP48FEB02_NV_GAIN_CTRL_I2C_SLAVE_REG_ADDR, > + (val1 & MCP48FEB02_GAIN_BITS_MASK) | > + (val & MCP48FEB02_NV_I2C_SLAVE_ADDR_MASK)); > + if (ret) > + return ret; > + > + return len; > +} > + Unneeded blank line. > +static IIO_DEVICE_ATTR_WO(store_eeprom, 0); ... > +static void mcp48feb02_init_scale(struct mcp48feb02_data *data, enum mcp48feb02_scale scale, > + int vref_uV, int scale_avail[]) > +{ > + u32 value_micro, value_int; > + u64 tmp; > + > + /* vref_uV should not be negative */ > + tmp = (u64)vref_uV * MILLI >> data->chip_features->resolution; If vref_uV is guaranteed to be less than ~33V, this code can be transformed to avoid 64-bit division. Hints: resolution is always great than 3; MILLI equals to 2³*5³. > + value_int = div_u64_rem(tmp, MICRO, &value_micro); > + scale_avail[scale * 2] = value_int; > + scale_avail[scale * 2 + 1] = value_micro; > +} Since it's kinda a common stuff, perhaps one wants to add a helper to include/linux/math.h. ... > +static int mcp48feb02_read_avail(struct iio_dev *indio_dev, struct iio_chan_spec const *ch, > + const int **vals, int *type, int *length, long info) > +{ > + struct mcp48feb02_data *data = iio_priv(indio_dev); > + > + switch (info) { > + case IIO_CHAN_INFO_SCALE: > + switch (ch->type) { > + case IIO_VOLTAGE: > + if (data->phys_channels >= 4 && (ch->address % 2)) > + *vals = data->scale_1; > + else > + *vals = data->scale; Actually, if you put the scales as int scales[2][2 * MCP48FEB02_MAX_SCALES_CH]; this will become as simple as if (data->phys_channels >= 4) *vals = data->scales[ch->address]; else *vals = data->scales[0]; OTOH, I am not sure if it can be always as *vals = data->scales[ch->address]; which would be the best approach. > + *length = 2 * MCP48FEB02_MAX_SCALES_CH; > + *type = IIO_VAL_INT_PLUS_MICRO; > + return IIO_AVAIL_LIST; > + default: > + return -EINVAL; > + } > + default: > + return -EINVAL; > + } > +} ... > +static void mcp48feb02_get_scale(int ch, struct mcp48feb02_data *data, int *val, int *val2) > +{ > + enum mcp48feb02_scale current_scale; > + > + if (data->chdata[ch].ref_mode == MCP48FEB02_VREF_VDD) > + current_scale = MCP48FEB02_SCALE_VDD; > + else if (data->chdata[ch].use_2x_gain) > + current_scale = MCP48FEB02_SCALE_GAIN_X2; > + else > + current_scale = MCP48FEB02_SCALE_GAIN_X1; > + > + if (data->phys_channels >= 4 && (ch % 2)) { > + *val = data->scale_1[current_scale * 2]; > + *val2 = data->scale_1[current_scale * 2 + 1]; > + } else { > + *val = data->scale[current_scale * 2]; > + *val2 = data->scale[current_scale * 2 + 1]; > + } Ditto. I.o.w. you can avoid (ch % 2) for good. > +} ... > +static int mcp48feb02_set_scale(struct mcp48feb02_data *data, int ch, int scale) > +{ > + int tmp_val, ret; Why is 'tmp_val' signed? > + ret = mcp48feb02_ch_scale(data, ch, scale); > + if (ret) > + return ret; > + > + if (scale == MCP48FEB02_SCALE_GAIN_X2) > + tmp_val = MCP48FEB02_GAIN_BIT_X2; > + else > + tmp_val = MCP48FEB02_GAIN_BIT_X1; > + > + ret = regmap_update_bits(data->regmap, MCP48FEB02_GAIN_CTRL_STATUS_REG_ADDR, > + DAC_GAIN_MASK(ch), DAC_GAIN_VAL(ch, tmp_val)); > + if (ret) > + return ret; > + > + data->chdata[ch].use_2x_gain = tmp_val; > + > + return 0; > +} ... > +static int mcp48feb02_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *ch, > + int val, int val2, long mask) > +{ > + struct mcp48feb02_data *data = iio_priv(indio_dev); > + int *tmp_scale, ret; > + > + guard(mutex)(&data->lock); > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + ret = regmap_write(data->regmap, REG_ADDR(ch->address), val); > + if (ret) > + return ret; > + > + data->chdata[ch->address].dac_data = val; > + return 0; > + case IIO_CHAN_INFO_SCALE: > + if (data->phys_channels >= 4 && (ch->address % 2)) > + tmp_scale = data->scale_1; > + else > + tmp_scale = data->scale; Same, (ch->address % 2) can be avoided. > + ret = mcp48feb02_check_scale(data, val, val2, tmp_scale); > + if (ret < 0) > + return ret; > + > + return mcp48feb02_set_scale(data, ch->address, ret); > + default: > + return -EINVAL; > + } > +} ... > +static int mcp48feb02_parse_fw(struct iio_dev *indio_dev, > + const struct mcp48feb02_features *chip_features) > +{ > + struct iio_chan_spec chanspec = mcp48febxx_ch_template; > + struct mcp48feb02_data *data = iio_priv(indio_dev); > + struct device *dev = regmap_get_device(data->regmap); > + struct iio_chan_spec *channels; > + u32 num_channels; > + u8 chan_idx = 0; Assignments like this are harder to maintain and prone to subtle mistakes in case the variable gets reused. Please, split it... > + guard(mutex)(&data->lock); > + > + num_channels = device_get_child_node_count(dev); > + if (num_channels > chip_features->phys_channels) > + return dev_err_probe(dev, -EINVAL, "More channels than the chip supports\n"); > + > + if (!num_channels) While this is standard pattern, I find == 0 is more explicit when we compare counters, but it's up to you and maintainers. > + return dev_err_probe(dev, -EINVAL, "No channel specified in the devicetree.\n"); > + > + channels = devm_kcalloc(dev, num_channels, sizeof(*channels), GFP_KERNEL); > + if (!channels) > + return -ENOMEM; ...to be here as chan_idx = 0; > + device_for_each_child_node_scoped(dev, child) { > + u32 reg = 0; Redundant assignment. "reg" is a mandatory property AFAICS from the below code. > + int ret; > + > + ret = fwnode_property_read_u32(child, "reg", ®); > + if (ret) > + return dev_err_probe(dev, ret, "Invalid channel number\n"); > + > + if (reg >= chip_features->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); Is atomic bit operation required here? > + ret = fwnode_property_read_string(child, "label", &data->labels[reg]); > + if (ret) > + return dev_err_probe(dev, ret, "%pfw: invalid label\n", > + fwnode_get_name(child)); Something is really wrong here. Please, fix accordingly. > + 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 = chip_features->phys_channels; > + > + data->vref_buffered = device_property_read_bool(dev, "microchip,vref-buffered"); > + if (chip_features->have_ext_vref1) > + data->vref1_buffered = device_property_read_bool(dev, "microchip,vref1-buffered"); Alternatively can be if (device_property_read_bool(dev, "microchip,vref1-buffered")) data->vref1_buffered = chip_features->have_ext_vref1; the difference is that vref1_buffered can be filled with "false", but I don't see if it can be true before that. You may stick with your variant to avoid this side effect. > + return 0; > +} ... > +static int mcp48feb02_init_ctrl_regs(struct mcp48feb02_data *data) > +{ > + unsigned int i, vref_ch, gain_ch, pd_ch; > + int ret; > + > + ret = regmap_read(data->regmap, MCP48FEB02_VREF_REG_ADDR, &vref_ch); > + if (ret) > + return ret; > + > + ret = regmap_read(data->regmap, MCP48FEB02_GAIN_CTRL_STATUS_REG_ADDR, &gain_ch); > + if (ret) > + return ret; > + > + ret = regmap_read(data->regmap, MCP48FEB02_POWER_DOWN_REG_ADDR, &pd_ch); > + if (ret) > + return ret; > + > + gain_ch = gain_ch & MCP48FEB02_GAIN_BITS_MASK; > + for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) { > + struct device *dev = regmap_get_device(data->regmap); > + unsigned int pd_tmp; > + > + data->chdata[i].ref_mode = (vref_ch >> (2 * i)) & MCP48FEB02_DAC_CTRL_MASK; > + data->chdata[i].use_2x_gain = (gain_ch >> i) & MCP48FEB02_GAIN_BIT_MASK; > + > + /* > + * Inform the user that the current voltage reference read from the volatile > + * register of the chip is different from the one specified in the device tree. > + * Considering that the user cannot have an external voltage reference connected > + * to the pin and select the internal Band Gap at the same time, in order to avoid > + * miscofiguring the reference voltage, the volatile register will not be written. > + * In order to overwrite the setting from volatile register with the one from the > + * device tree, the user needs to write the chosen scale. > + */ > + switch (data->chdata[i].ref_mode) { > + case MCP48FEB02_INTERNAL_BAND_GAP: > + if (data->phys_channels >= 4 && (i % 2) && data->use_vref1) { > + dev_dbg(dev, "ch[%u]: was configured to use internal band gap", i); > + dev_dbg(dev, "ch[%u]: reference voltage set to VREF1", i); > + break; > + } > + if ((data->phys_channels < 4 || (data->phys_channels >= 4 && !(i % 2))) && > + data->use_vref) { I don't see how these two conditionals can be run both. > + dev_dbg(dev, "ch[%u]: was configured to use internal band gap", i); > + dev_dbg(dev, "ch[%u]: reference voltage set to VREF", i); > + break; > + } With that in mind, the above can be simplified a bit. if (data->use_vref && ((data->phys_channels >= 4 && !(i % 2)) || (data->phys_channels < 4))) { dev_dbg(dev, "ch[%u]: was configured to use internal band gap\n", i); dev_dbg(dev, "ch[%u]: reference voltage set to Vref\n", i); } else if (data->use_vref1 && data->phys_channels >= 4 && (i % 2)) { dev_dbg(dev, "ch[%u]: was configured to use internal band gap\n", i); dev_dbg(dev, "ch[%u]: reference voltage set to Vref1\n", i); } The conditionals were reshuffled to make it shorter and easier to compare (yes, there is a pair of unneeded parentheses for the sake of good looking code, a.k.a. readability). Also note, the messages were missing trailing '\n'; I lowered REF --> ref in them. > + break; > + case MCP48FEB02_EXTERNAL_VREF_UNBUFFERED: > + case MCP48FEB02_EXTERNAL_VREF_BUFFERED: > + if (data->phys_channels >= 4 && (i % 2) && !data->use_vref1) { > + dev_dbg(dev, "ch[%u]: was configured to use VREF1", i); > + dev_dbg(dev, > + "ch[%u]: reference voltage set to internal band gap", i); > + break; > + } > + if ((data->phys_channels < 4 || (data->phys_channels >= 4 && !(i % 2))) && > + !data->use_vref) { > + dev_dbg(dev, "ch[%u]: was configured to use VREF", i); > + dev_dbg(dev, > + "ch[%u]: reference voltage set to internal band gap", i); > + break; > + } > + break; Ditto. > + } > + > + pd_tmp = (pd_ch >> (2 * i)) & MCP48FEB02_DAC_CTRL_MASK; > + data->chdata[i].powerdown_mode = pd_tmp ? (pd_tmp - 1) : pd_tmp; > + data->chdata[i].powerdown = !!(data->chdata[i].powerdown_mode); > + } > + > + return 0; > +} ... > +static int mcp48feb02_probe(struct spi_device *spi) > +{ > + const struct mcp48feb02_features *chip_features; > + struct device *dev = &spi->dev; > + struct mcp48feb02_data *data; > + struct iio_dev *indio_dev; > + int vref1_uV = 0; > + int vref_uV = 0; Please, split the assignments (the rationale was given somewhere above). > + int vdd_uV; > + int ret; > + > + indio_dev = devm_iio_device_alloc(dev, sizeof(*data)); > + if (!indio_dev) > + return -ENOMEM; > + > + data = iio_priv(indio_dev); > + > + chip_features = spi_get_device_match_data(spi); > + if (!chip_features) > + return -EINVAL; > + > + data->chip_features = chip_features; > + > + if (chip_features->have_eeprom) { > + data->regmap = devm_regmap_init_spi(spi, &mcp48feb02_regmap_config); > + indio_dev->info = &mcp48feb02_info; > + } else { > + data->regmap = devm_regmap_init_spi(spi, &mcp48fvb02_regmap_config); > + indio_dev->info = &mcp48fvb02_info; > + } > + if (IS_ERR(data->regmap)) > + return dev_err_probe(dev, PTR_ERR(data->regmap), "Error initializing spi regmap\n"); SPI > + indio_dev->name = chip_features->name; > + > + ret = mcp48feb02_parse_fw(indio_dev, chip_features); > + if (ret) > + return dev_err_probe(dev, ret, "Error parsing firmware data\n"); > + > + ret = devm_mutex_init(dev, &data->lock); > + if (ret) > + return ret; > + > + ret = devm_regulator_get_enable_read_voltage(dev, "vdd"); > + if (ret < 0) > + return ret; > + > + vdd_uV = ret; > + > + ret = devm_regulator_get_enable_read_voltage(dev, "vref"); > + if (ret > 0) { > + vref_uV = ret; > + data->use_vref = true; > + } else { > + dev_dbg(dev, "using internal band gap as voltage reference.\n"); > + dev_dbg(dev, "External Vref is unavailable.\n"); > + } > + > + if (chip_features->have_ext_vref1) { > + ret = devm_regulator_get_enable_read_voltage(dev, "vref1"); > + if (ret > 0) { > + vref1_uV = ret; > + data->use_vref1 = true; > + } else { > + dev_dbg(dev, "using internal band gap as voltage reference 1.\n"); > + dev_dbg(dev, "External Vref1 is unavailable.\n"); > + } > + } > + > + ret = mcp48feb02_init_ctrl_regs(data); > + if (ret) > + return dev_err_probe(dev, ret, "Error initialising vref register\n"); > + > + ret = mcp48feb02_init_ch_scales(data, vdd_uV, vref_uV, vref1_uV); > + if (ret) > + return ret; > + > + return devm_iio_device_register(dev, indio_dev); > +} -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] iio: dac: add support for Microchip MCP48FEB02 2026-02-12 14:42 ` Andy Shevchenko @ 2026-02-16 14:29 ` Ariana.Lazar 2026-02-17 7:54 ` Andy Shevchenko 0 siblings, 1 reply; 18+ messages in thread From: Ariana.Lazar @ 2026-02-16 14:29 UTC (permalink / raw) To: andriy.shevchenko Cc: dlechner, nuno.sa, linux-iio, devicetree, robh, jic23, andy, krzk+dt, linux-kernel, conor+dt Hello Andy, please see my comments below > > > +static int mcp48feb02_read_avail(struct iio_dev *indio_dev, struct > > iio_chan_spec const *ch, > > + const int **vals, int *type, int > > *length, long info) > > +{ > > + struct mcp48feb02_data *data = iio_priv(indio_dev); > > + > > + switch (info) { > > + case IIO_CHAN_INFO_SCALE: > > + switch (ch->type) { > > + case IIO_VOLTAGE: > > + if (data->phys_channels >= 4 && (ch->address > > % 2)) > > + *vals = data->scale_1; > > + else > > + *vals = data->scale; > > Actually, if you put the scales as > > int scales[2][2 * MCP48FEB02_MAX_SCALES_CH]; > > this will become as simple as > > if (data->phys_channels >= 4) > *vals = data->scales[ch->address]; > else > *vals = data->scales[0]; > > OTOH, I am not sure if it can be always as > > *vals = data->scales[ch->address]; > > which would be the best approach. > > > > I am not quite sure I have understood your point of view. In order to remove the channel parity check, I would have to declare int scales[MCP48FEB02_MAX_CH][2 * MCP48FEB02_MAX_SCALES_CH] (int scales[8][6]) regardless of device's number of channels and number of voltage references. This will be quite a lot unnecessary space allocated compared to using only two arrays of [2 * MCP48FEB02_MAX_SCALES_CH]. Another way to avoid these checks is to use a dynamically allocated array of scales. Each member points to an array of [2 * MCP48FEB02_MAX_SCALES_CH] and stores corresponding scale values for each channel, while allowing to allocate the actual number of channels the device has rather than the maximum. Please tell me which version you prefer. Best regards, Ariana ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] iio: dac: add support for Microchip MCP48FEB02 2026-02-16 14:29 ` Ariana.Lazar @ 2026-02-17 7:54 ` Andy Shevchenko 2026-02-17 11:38 ` Ariana.Lazar 0 siblings, 1 reply; 18+ messages in thread From: Andy Shevchenko @ 2026-02-17 7:54 UTC (permalink / raw) To: Ariana.Lazar Cc: dlechner, nuno.sa, linux-iio, devicetree, robh, jic23, andy, krzk+dt, linux-kernel, conor+dt On Mon, Feb 16, 2026 at 02:29:19PM +0000, Ariana.Lazar@microchip.com wrote: ... > > > +static int mcp48feb02_read_avail(struct iio_dev *indio_dev, struct > > > iio_chan_spec const *ch, > > > + const int **vals, int *type, int > > > *length, long info) > > > +{ > > > + struct mcp48feb02_data *data = iio_priv(indio_dev); > > > + > > > + switch (info) { > > > + case IIO_CHAN_INFO_SCALE: > > > + switch (ch->type) { > > > + case IIO_VOLTAGE: > > > + if (data->phys_channels >= 4 && (ch->address > > > % 2)) > > > + *vals = data->scale_1; > > > + else > > > + *vals = data->scale; > > > > Actually, if you put the scales as > > > > int scales[2][2 * MCP48FEB02_MAX_SCALES_CH]; > > > > this will become as simple as > > > > if (data->phys_channels >= 4) > > *vals = data->scales[ch->address]; > > else > > *vals = data->scales[0]; > > > > OTOH, I am not sure if it can be always as > > > > *vals = data->scales[ch->address]; > > > > which would be the best approach. > > I am not quite sure I have understood your point of view. In order to > remove the channel parity check, I would have to declare int > scales[MCP48FEB02_MAX_CH][2 * MCP48FEB02_MAX_SCALES_CH] (int > scales[8][6]) > regardless of device's number of channels and number of voltage > references. This will be quite a lot unnecessary space allocated > compared to using only two arrays of [2 * MCP48FEB02_MAX_SCALES_CH]. > > Another way to avoid these checks is to use a dynamically allocated > array of scales. Each member points to an array of [2 * > MCP48FEB02_MAX_SCALES_CH] > and stores corresponding scale values for each channel, while allowing > to allocate the actual number of channels the device has rather than > the maximum. > > Please tell me which version you prefer. I think something like a second variant. But as it seems going to be an agreement that this driver is not needed and rather we need to refactor existing one to add the support for SPI chips, these comments won't make much value. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] iio: dac: add support for Microchip MCP48FEB02 2026-02-17 7:54 ` Andy Shevchenko @ 2026-02-17 11:38 ` Ariana.Lazar 2026-02-17 12:12 ` Andy Shevchenko 0 siblings, 1 reply; 18+ messages in thread From: Ariana.Lazar @ 2026-02-17 11:38 UTC (permalink / raw) To: andriy.shevchenko Cc: dlechner, nuno.sa, linux-iio, devicetree, robh, jic23, andy, krzk+dt, linux-kernel, conor+dt Hello Andy, > I think something like a second variant. But as it seems going to be > an > agreement that this driver is not needed and rather we need to > refactor > existing one to add the support for SPI chips, these comments won't > make > much value. > > -- Thank you fo the review. I think these comments are revelent because that code is common for both drivers, the SPI and I2C. I will refactor both drivers to be merged into a single one and in order to avoid multiple code refactoring, if you agree, I will implement those changes as well. Best regards, Ariana > With Best Regards, > Andy Shevchenko > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] iio: dac: add support for Microchip MCP48FEB02 2026-02-17 11:38 ` Ariana.Lazar @ 2026-02-17 12:12 ` Andy Shevchenko 0 siblings, 0 replies; 18+ messages in thread From: Andy Shevchenko @ 2026-02-17 12:12 UTC (permalink / raw) To: Ariana.Lazar Cc: dlechner, nuno.sa, linux-iio, devicetree, robh, jic23, andy, krzk+dt, linux-kernel, conor+dt On Tue, Feb 17, 2026 at 11:38:10AM +0000, Ariana.Lazar@microchip.com wrote: > > I think something like a second variant. But as it seems going to be > > an > > agreement that this driver is not needed and rather we need to > > refactor > > existing one to add the support for SPI chips, these comments won't > > make > > much value. > > > > -- > Thank you fo the review. I think these comments are revelent because > that code is common for both drivers, the SPI and I2C. > I will refactor both drivers to be merged into a single one and in > order to avoid multiple code refactoring, if you agree, I will > implement those changes as well. Sure, if you have time for that. I'm not insisting... -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] iio: dac: add support for Microchip MCP48FEB02 2026-02-12 12:48 ` [PATCH 2/2] " Ariana Lazar 2026-02-12 14:42 ` Andy Shevchenko @ 2026-02-15 17:58 ` Jonathan Cameron 1 sibling, 0 replies; 18+ messages in thread From: Jonathan Cameron @ 2026-02-15 17:58 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 Thu, 12 Feb 2026 14:48:35 +0200 Ariana Lazar <ariana.lazar@microchip.com> wrote: > This is the iio driver for Microchip MCP48FxBy1/2/4/8 series of buffered > voltage output Digital-to-Analog Converters with nonvolatile or volatile > memory and an SPI 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> Hi Ariana, Given the much larger outstanding question on whether this can share a driver with other similar parts I only took a very quick look at this version. Comments inline. Thanks, Jonathan > diff --git a/drivers/iio/dac/mcp48feb02.c b/drivers/iio/dac/mcp48feb02.c > new file mode 100644 > index 0000000000000000000000000000000000000000..20955f77053329a9c385f55c7314032eb6b04dfd > --- /dev/null > +++ b/drivers/iio/dac/mcp48feb02.c > + > +static int mcp48feb02_init_ctrl_regs(struct mcp48feb02_data *data) > +{ > + unsigned int i, vref_ch, gain_ch, pd_ch; > + int ret; > + > + ret = regmap_read(data->regmap, MCP48FEB02_VREF_REG_ADDR, &vref_ch); > + if (ret) > + return ret; > + > + ret = regmap_read(data->regmap, MCP48FEB02_GAIN_CTRL_STATUS_REG_ADDR, &gain_ch); > + if (ret) > + return ret; > + > + ret = regmap_read(data->regmap, MCP48FEB02_POWER_DOWN_REG_ADDR, &pd_ch); > + if (ret) > + return ret; > + > + gain_ch = gain_ch & MCP48FEB02_GAIN_BITS_MASK; > + for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) { > + struct device *dev = regmap_get_device(data->regmap); > + unsigned int pd_tmp; > + > + data->chdata[i].ref_mode = (vref_ch >> (2 * i)) & MCP48FEB02_DAC_CTRL_MASK; > + data->chdata[i].use_2x_gain = (gain_ch >> i) & MCP48FEB02_GAIN_BIT_MASK; > + > + /* > + * Inform the user that the current voltage reference read from the volatile > + * register of the chip is different from the one specified in the device tree. > + * Considering that the user cannot have an external voltage reference connected > + * to the pin and select the internal Band Gap at the same time, in order to avoid > + * miscofiguring the reference voltage, the volatile register will not be written. Spell check comments. misconfiguring > + * In order to overwrite the setting from volatile register with the one from the > + * device tree, the user needs to write the chosen scale. I'm a little unsure of why we need this extra gate on updating things to match the device tree provided config. Why should the volatile register at this point match what DT says? If it does seems to me we should be noisier about it than dev_dbg() > + */ > + switch (data->chdata[i].ref_mode) { > + case MCP48FEB02_INTERNAL_BAND_GAP: > + if (data->phys_channels >= 4 && (i % 2) && data->use_vref1) { > + dev_dbg(dev, "ch[%u]: was configured to use internal band gap", i); > + dev_dbg(dev, "ch[%u]: reference voltage set to VREF1", i); > + break; > + } > + if ((data->phys_channels < 4 || (data->phys_channels >= 4 && !(i % 2))) && > + data->use_vref) { > + dev_dbg(dev, "ch[%u]: was configured to use internal band gap", i); > + dev_dbg(dev, "ch[%u]: reference voltage set to VREF", i); > + break; > + } ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/2] Add support for Microchip MCP48FxBy1/2/4/8 DAC with an SPI Interface 2026-02-12 12:48 [PATCH 0/2] Add support for Microchip MCP48FxBy1/2/4/8 DAC with an SPI Interface Ariana Lazar 2026-02-12 12:48 ` [PATCH 1/2] dt-bindings: iio: dac: add support for Microchip MCP48FEB02 Ariana Lazar 2026-02-12 12:48 ` [PATCH 2/2] " Ariana Lazar @ 2026-02-12 13:39 ` Andy Shevchenko 2026-02-12 17:58 ` Conor Dooley 2 siblings, 1 reply; 18+ messages in thread From: Andy Shevchenko @ 2026-02-12 13:39 UTC (permalink / raw) To: Ariana Lazar Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio, devicetree, linux-kernel On Thu, Feb 12, 2026 at 02:48:33PM +0200, Ariana Lazar wrote: > Add support for Microchip MCP48FxBy1/2/4/8 series of buffered voltage > output Digital-to-Analog converters with an SPI Interface. This driver > covers the following part numbers: > - With nonvolatile memory: > - MCP48FEB01, MCP48FEB02, MCP48FEB04, MCP48FEB08, > MCP48FEB11, MCP48FEB12, MCP48FEB14, MCP48FEB18, > MCP48FEB21, MCP48FEB22, MCP48FEB24, MCP48FEB28 > - With volatile memory: > - MCP48FVB01, MCP48FVB02, MCP48FVB04, MCP48FVB08, > MCP48FVB11, MCP48FVB12, MCP48FVB14, MCP48FVB18, > MCP48FVB21, MCP48FVB22, MCP48FVB24, MCP48FVB28 > > The families support up to 8 output channels. The devices can be 8-bit, > 10-bit and 12-bit resolution. Is it really v1? I am under impression that I have seen this patch already several times... -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/2] Add support for Microchip MCP48FxBy1/2/4/8 DAC with an SPI Interface 2026-02-12 13:39 ` [PATCH 0/2] Add support for Microchip MCP48FxBy1/2/4/8 DAC with an SPI Interface Andy Shevchenko @ 2026-02-12 17:58 ` Conor Dooley 0 siblings, 0 replies; 18+ messages in thread From: Conor Dooley @ 2026-02-12 17:58 UTC (permalink / raw) To: Andy Shevchenko Cc: Ariana Lazar, Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1090 bytes --] On Thu, Feb 12, 2026 at 03:39:19PM +0200, Andy Shevchenko wrote: > On Thu, Feb 12, 2026 at 02:48:33PM +0200, Ariana Lazar wrote: > > Add support for Microchip MCP48FxBy1/2/4/8 series of buffered voltage > > output Digital-to-Analog converters with an SPI Interface. This driver > > covers the following part numbers: > > - With nonvolatile memory: > > - MCP48FEB01, MCP48FEB02, MCP48FEB04, MCP48FEB08, > > MCP48FEB11, MCP48FEB12, MCP48FEB14, MCP48FEB18, > > MCP48FEB21, MCP48FEB22, MCP48FEB24, MCP48FEB28 > > - With volatile memory: > > - MCP48FVB01, MCP48FVB02, MCP48FVB04, MCP48FVB08, > > MCP48FVB11, MCP48FVB12, MCP48FVB14, MCP48FVB18, > > MCP48FVB21, MCP48FVB22, MCP48FVB24, MCP48FVB28 > > > > The families support up to 8 output channels. The devices can be 8-bit, > > 10-bit and 12-bit resolution. > > Is it really v1? I am under impression that I have seen this patch already > several times... Me too but I think it's a subtle difference between mcp47febxx and mcp48febxx that's probably lost in very similar looking part names. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2026-02-17 12:12 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-02-12 12:48 [PATCH 0/2] Add support for Microchip MCP48FxBy1/2/4/8 DAC with an SPI Interface Ariana Lazar 2026-02-12 12:48 ` [PATCH 1/2] dt-bindings: iio: dac: add support for Microchip MCP48FEB02 Ariana Lazar 2026-02-12 17:31 ` Rob Herring (Arm) 2026-02-12 18:00 ` Conor Dooley 2026-02-12 20:04 ` Andy Shevchenko 2026-02-16 13:31 ` Ariana.Lazar 2026-02-16 15:37 ` David Lechner 2026-02-16 17:34 ` Conor Dooley 2026-02-16 18:38 ` David Lechner 2026-02-12 12:48 ` [PATCH 2/2] " Ariana Lazar 2026-02-12 14:42 ` Andy Shevchenko 2026-02-16 14:29 ` Ariana.Lazar 2026-02-17 7:54 ` Andy Shevchenko 2026-02-17 11:38 ` Ariana.Lazar 2026-02-17 12:12 ` Andy Shevchenko 2026-02-15 17:58 ` Jonathan Cameron 2026-02-12 13:39 ` [PATCH 0/2] Add support for Microchip MCP48FxBy1/2/4/8 DAC with an SPI Interface Andy Shevchenko 2026-02-12 17:58 ` Conor Dooley
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox