* [PATCH v2 0/2] iio: dac: mcp47a1: add support for new device
@ 2026-07-27 18:31 Joshua Crofts
2026-07-27 18:31 ` [PATCH v2 1/2] dt-bindings: iio: dac: add support for mcp47a1 Joshua Crofts
2026-07-27 18:31 ` [PATCH v2 2/2] iio: dac: mcp47a1: add support for new device Joshua Crofts
0 siblings, 2 replies; 8+ messages in thread
From: Joshua Crofts @ 2026-07-27 18:31 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-iio, devicetree, linux-kernel, Joshua Crofts, Conor Dooley
This patch series adds support for the MCP47A1 DAC.
The Microchip MCP47A1 is a volatile 6-bit Digital-to-Analog converter
which communicates via I2C. It is a low-power, low-cost DAC commonly
used in embedded systems.
Reasons for adding a new driver:
- Existing Microchip DACs use a more complicated set of commands
whereas the MCP47A1 communicates via SMBus using only one command.
- The closest Microchip DAC already in the tree - the MCP4725 - uses
an EEPROM while the MCP47A1 is volatile, meaning that altering
the driver of the former would worsen the code flow.
- The MCP47A1 maps cleanly to a standard regmap.
The driver was tested on a Raspberry Pi 4 using a breakout board with
the MCP47A1.
The datasheet can be viewed here:
https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/25154A.pdf
Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
Changes in v2:
- dt-bindings: add slave address enum to dt-binding
- dt-bindings: remove unnecessary example
- move driver from regmap to i2c_smbus_* functions to reduce overhead
- remove read test on probe as it's too unstable
- add 20us delay on startup
- style changes
- remove excess mailing list entry from MAINTAINERS
- Link to v1: https://lore.kernel.org/r/20260721-mcp47a1-add-support-v1-0-da045a2567e3@gmail.com
---
Joshua Crofts (2):
dt-bindings: iio: dac: add support for mcp47a1
iio: dac: mcp47a1: add support for new device
.../bindings/iio/dac/microchip,mcp47a1.yaml | 50 ++++++
MAINTAINERS | 6 +
drivers/iio/dac/Kconfig | 10 ++
drivers/iio/dac/Makefile | 1 +
drivers/iio/dac/mcp47a1.c | 169 +++++++++++++++++++++
5 files changed, 236 insertions(+)
---
base-commit: 0506462caaecb179708657142575823177c83783
change-id: 20260711-mcp47a1-add-support-3af1248f13a5
Best regards,
--
Kind regards,
Joshua Crofts
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v2 1/2] dt-bindings: iio: dac: add support for mcp47a1 2026-07-27 18:31 [PATCH v2 0/2] iio: dac: mcp47a1: add support for new device Joshua Crofts @ 2026-07-27 18:31 ` Joshua Crofts 2026-07-27 18:31 ` [PATCH v2 2/2] iio: dac: mcp47a1: add support for new device Joshua Crofts 1 sibling, 0 replies; 8+ messages in thread From: Joshua Crofts @ 2026-07-27 18:31 UTC (permalink / raw) To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-iio, devicetree, linux-kernel, Joshua Crofts, Conor Dooley The Microchip MCP47A1 is a 6-bit volatile Digital-to-Analog converter which communicates via I2C. Reviewed-by: Conor Dooley <conor.dooley@microchip.com> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com> --- .../bindings/iio/dac/microchip,mcp47a1.yaml | 50 ++++++++++++++++++++++ MAINTAINERS | 5 +++ 2 files changed, 55 insertions(+) diff --git a/Documentation/devicetree/bindings/iio/dac/microchip,mcp47a1.yaml b/Documentation/devicetree/bindings/iio/dac/microchip,mcp47a1.yaml new file mode 100644 index 000000000000..b181fe73dce8 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/dac/microchip,mcp47a1.yaml @@ -0,0 +1,50 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- + +$id: http://devicetree.org/schemas/iio/dac/microchip,mcp47a1.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Microchip MCP47A1 DAC + +maintainers: + - Joshua Crofts <joshua.crofts1@gmail.com> + +description: | + The Microchip MCP47A1 is a 6-bit single output volatile DAC. + This device can have different IDs (0x2e and 0x3e). + https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/25154A.pdf + +properties: + compatible: + const: microchip,mcp47a1 + + reg: + items: + - enum: [0x2e, 0x3e] + + vref-supply: true + + vdd-supply: true + +required: + - compatible + - reg + - vref-supply + - vdd-supply + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + dac@2e { + compatible = "microchip,mcp47a1"; + reg = <0x2e>; + vref-supply = <&vref_regulator>; + vdd-supply = <&vdd_regulator>; + }; + }; diff --git a/MAINTAINERS b/MAINTAINERS index bcdc00999ef8..8a0b23418a64 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -17699,6 +17699,11 @@ S: Maintained F: Documentation/devicetree/bindings/iio/adc/microchip,mcp3911.yaml F: drivers/iio/adc/mcp3911.c +MICROCHIP MCP47A1 DAC DRIVER +M: Joshua Crofts <joshua.crofts1@gmail.com> +S: Maintained +F: Documentation/devicetree/bindings/iio/dac/microchip,mcp47a1.yaml + MICROCHIP MCP9982 TEMPERATURE DRIVER M: Victor Duicu <victor.duicu@microchip.com> L: linux-hwmon@vger.kernel.org -- 2.55.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] iio: dac: mcp47a1: add support for new device 2026-07-27 18:31 [PATCH v2 0/2] iio: dac: mcp47a1: add support for new device Joshua Crofts 2026-07-27 18:31 ` [PATCH v2 1/2] dt-bindings: iio: dac: add support for mcp47a1 Joshua Crofts @ 2026-07-27 18:31 ` Joshua Crofts 2026-07-27 18:42 ` sashiko-bot 2026-08-01 2:31 ` Jonathan Cameron 1 sibling, 2 replies; 8+ messages in thread From: Joshua Crofts @ 2026-07-27 18:31 UTC (permalink / raw) To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-iio, devicetree, linux-kernel, Joshua Crofts The Microchip MCP47A1 is a 6-bit volatile Digital-to-Analog converter which communicates via I2C. Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com> --- MAINTAINERS | 1 + drivers/iio/dac/Kconfig | 10 +++ drivers/iio/dac/Makefile | 1 + drivers/iio/dac/mcp47a1.c | 169 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 181 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 8a0b23418a64..552ced65cf75 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -17703,6 +17703,7 @@ MICROCHIP MCP47A1 DAC DRIVER M: Joshua Crofts <joshua.crofts1@gmail.com> S: Maintained F: Documentation/devicetree/bindings/iio/dac/microchip,mcp47a1.yaml +F: drivers/iio/dac/mcp47a1.c MICROCHIP MCP9982 TEMPERATURE DRIVER M: Victor Duicu <victor.duicu@microchip.com> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig index d6d560c09e25..1d00cd911826 100644 --- a/drivers/iio/dac/Kconfig +++ b/drivers/iio/dac/Kconfig @@ -565,6 +565,16 @@ config MCP4728 To compile this driver as a module, choose M here: the module will be called mcp4728. +config MCP47A1 + tristate "MCP47A1 DAC driver" + depends on I2C + help + Say Y here if you want to build a driver for the Microchip + MCP47A1 digital-to-analog converter with an I2C interface. + + To compile this driver as a module, choose M here: the module + will be called mcp47a1. + config MCP47FEB02 tristate "MCP47F(E/V)B01/02/04/08/11/12/14/18/21/22/24/28 DAC driver" depends on I2C diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile index 5d20d37e44ce..992f8930f95c 100644 --- a/drivers/iio/dac/Makefile +++ b/drivers/iio/dac/Makefile @@ -55,6 +55,7 @@ obj-$(CONFIG_MAX5821) += max5821.o obj-$(CONFIG_MCF54415_DAC) += mcf54415_dac.o obj-$(CONFIG_MCP4725) += mcp4725.o obj-$(CONFIG_MCP4728) += mcp4728.o +obj-$(CONFIG_MCP47A1) += mcp47a1.o obj-$(CONFIG_MCP47FEB02) += mcp47feb02.o obj-$(CONFIG_MCP4821) += mcp4821.o obj-$(CONFIG_MCP4922) += mcp4922.o diff --git a/drivers/iio/dac/mcp47a1.c b/drivers/iio/dac/mcp47a1.c new file mode 100644 index 000000000000..140e93ff2ba0 --- /dev/null +++ b/drivers/iio/dac/mcp47a1.c @@ -0,0 +1,169 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Microchip MCP47A1 DAC driver + * + * Copyright (c) 2026 Joshua Crofts <joshua.crofts1@gmail.com> + * + * Datasheet: https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/25154A.pdf + */ + +#include <linux/array_size.h> +#include <linux/delay.h> +#include <linux/err.h> +#include <linux/i2c.h> +#include <linux/module.h> +#include <linux/regulator/consumer.h> +#include <linux/types.h> +#include <linux/units.h> + +#include <linux/iio/iio.h> + +#define MCP47A1_REG_MAX 0x40 +#define MCP47A1_CMD_CODE 0x00 +#define MCP47A1_MAX_STEP 63 + +struct mcp47a1_data { + struct i2c_client *client; + int vref_mV; +}; + +static const int mcp47a1_raw_avail[] = { 0, 1, MCP47A1_MAX_STEP }; + +static const struct iio_chan_spec mcp47a1_channels[] = { + { + .type = IIO_VOLTAGE, + .indexed = 1, + .output = 1, + .channel = 0, + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), + .info_mask_separate_available = BIT(IIO_CHAN_INFO_RAW), + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), + }, +}; + +static int mcp47a1_write(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int val, int val2, long mask) +{ + struct mcp47a1_data *data = iio_priv(indio_dev); + + switch (mask) { + case IIO_CHAN_INFO_RAW: + if (val < 0 || val > MCP47A1_MAX_STEP) + return -EINVAL; + + return i2c_smbus_write_byte_data(data->client, MCP47A1_CMD_CODE, + val); + default: + return -EINVAL; + } +} + +static int mcp47a1_read(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int *val, int *val2, long mask) +{ + struct mcp47a1_data *data = iio_priv(indio_dev); + int ret; + + switch (mask) { + case IIO_CHAN_INFO_RAW: + ret = i2c_smbus_read_byte_data(data->client, MCP47A1_CMD_CODE); + if (ret < 0) + return ret; + + *val = ret; + + return IIO_VAL_INT; + case IIO_CHAN_INFO_SCALE: + *val = data->vref_mV; + *val2 = MCP47A1_MAX_STEP; + + return IIO_VAL_FRACTIONAL; + default: + return -EINVAL; + } +} + +static int mcp47a1_read_avail(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + const int **vals, int *type, int *length, + long mask) +{ + switch (mask) { + case IIO_CHAN_INFO_RAW: + *vals = mcp47a1_raw_avail; + *type = IIO_VAL_INT; + return IIO_AVAIL_RANGE; + default: + return -EINVAL; + } +} + +static const struct iio_info mcp47a1_info = { + .write_raw = mcp47a1_write, + .read_raw = mcp47a1_read, + .read_avail = mcp47a1_read_avail, +}; + +static int mcp47a1_probe(struct i2c_client *client) +{ + struct device *dev = &client->dev; + struct mcp47a1_data *data; + struct iio_dev *indio_dev; + int ret; + + indio_dev = devm_iio_device_alloc(dev, sizeof(*data)); + if (!indio_dev) + return -ENOMEM; + + data = iio_priv(indio_dev); + data->client = client; + + ret = devm_regulator_get_enable(dev, "vdd"); + if (ret) + return dev_err_probe(dev, ret, "Failed to enable regulator\n"); + + /* Delay after device exits reset state (see AC/DC characteristics) */ + fsleep(20); + + ret = devm_regulator_get_enable_read_voltage(dev, "vref"); + if (ret < 0) + return dev_err_probe(dev, ret, "Failed to read vref\n"); + + data->vref_mV = ret / (MICRO / MILLI); + + indio_dev->name = "mcp47a1"; + indio_dev->modes = INDIO_DIRECT_MODE; + indio_dev->info = &mcp47a1_info; + indio_dev->channels = mcp47a1_channels; + indio_dev->num_channels = ARRAY_SIZE(mcp47a1_channels); + + return devm_iio_device_register(dev, indio_dev); +} + +static const struct of_device_id mcp47a1_of_match[] = { + { .compatible = "microchip,mcp47a1" }, + { } +}; +MODULE_DEVICE_TABLE(of, mcp47a1_of_match); + +static const struct i2c_device_id mcp47a1_id[] = { + { .name = "mcp47a1" }, + { } +}; +MODULE_DEVICE_TABLE(i2c, mcp47a1_id); + +static struct i2c_driver mcp47a1_driver = { + .driver = { + .name = "mcp47a1", + .of_match_table = mcp47a1_of_match, + }, + .probe = mcp47a1_probe, + .id_table = mcp47a1_id, +}; +module_i2c_driver(mcp47a1_driver); + +MODULE_AUTHOR("Joshua Crofts <joshua.crofts1@gmail.com>"); +MODULE_DESCRIPTION("MCP47A1 DAC"); +MODULE_LICENSE("GPL"); -- 2.55.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] iio: dac: mcp47a1: add support for new device 2026-07-27 18:31 ` [PATCH v2 2/2] iio: dac: mcp47a1: add support for new device Joshua Crofts @ 2026-07-27 18:42 ` sashiko-bot 2026-07-28 13:15 ` Joshua Crofts 2026-08-01 2:31 ` Jonathan Cameron 1 sibling, 1 reply; 8+ messages in thread From: sashiko-bot @ 2026-07-27 18:42 UTC (permalink / raw) To: Joshua Crofts; +Cc: devicetree, conor+dt, robh Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Potential off-by-one in DAC scale calculation denominator. -- --- Patch [2]: [PATCH v2 2/2] iio: dac: mcp47a1: add support for new device --- commit 38bec2fbb020c3f749c52a26e5cc3e78e72eeaf5 Author: Joshua Crofts <joshua.crofts1@gmail.com> iio: dac: mcp47a1: add support for new device The Microchip MCP47A1 is a 6-bit volatile Digital-to-Analog converter which communicates via I2C. > diff --git a/drivers/iio/dac/mcp47a1.c b/drivers/iio/dac/mcp47a1.c > new file mode 100644 > index 0000000000000..140e93ff2ba01 > --- /dev/null > +++ b/drivers/iio/dac/mcp47a1.c [ ... ] > +static int mcp47a1_read(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int *val, int *val2, long mask) > +{ > + struct mcp47a1_data *data = iio_priv(indio_dev); > + int ret; > + > + switch (mask) { [ ... ] > + case IIO_CHAN_INFO_SCALE: > + *val = data->vref_mV; > + *val2 = MCP47A1_MAX_STEP; [Severity: Medium] Does this code use the correct denominator for the scale calculation? Since this is a 6-bit DAC, shouldn't the scale denominator be 64 (2^6) instead of the maximum step value of 63? Using MCP47A1_MAX_STEP (63) here introduces an off-by-one error that could cause userspace to calculate output voltages with a ~1.5% LSB scaling error when reading the sysfs attribute. > + > + return IIO_VAL_FRACTIONAL; > + default: > + return -EINVAL; > + } > +} -- Sashiko AI review · https://sashiko.dev/#/patchset/20260727-mcp47a1-add-support-v2-0-d4591a2ce69e@gmail.com?part=2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] iio: dac: mcp47a1: add support for new device 2026-07-27 18:42 ` sashiko-bot @ 2026-07-28 13:15 ` Joshua Crofts 2026-08-01 2:30 ` Jonathan Cameron 0 siblings, 1 reply; 8+ messages in thread From: Joshua Crofts @ 2026-07-28 13:15 UTC (permalink / raw) To: sashiko-bot Cc: sashiko-reviews, devicetree, conor+dt, robh, linux-iio, jic23, andy, nuno.sa, dlechner On Mon, 27 Jul 2026 18:42:26 +0000 sashiko-bot@kernel.org wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [Medium] Potential off-by-one in DAC scale calculation denominator. > -- Responding to Sashiko's comment. > --- Patch [2]: [PATCH v2 2/2] iio: dac: mcp47a1: add support for new device --- > commit 38bec2fbb020c3f749c52a26e5cc3e78e72eeaf5 > Author: Joshua Crofts <joshua.crofts1@gmail.com> > > iio: dac: mcp47a1: add support for new device > > The Microchip MCP47A1 is a 6-bit volatile Digital-to-Analog converter > which communicates via I2C. > > > diff --git a/drivers/iio/dac/mcp47a1.c b/drivers/iio/dac/mcp47a1.c > > new file mode 100644 > > index 0000000000000..140e93ff2ba01 > > --- /dev/null > > +++ b/drivers/iio/dac/mcp47a1.c > [ ... ] > > +static int mcp47a1_read(struct iio_dev *indio_dev, > > + struct iio_chan_spec const *chan, > > + int *val, int *val2, long mask) > > +{ > > + struct mcp47a1_data *data = iio_priv(indio_dev); > > + int ret; > > + > > + switch (mask) { > [ ... ] > > + case IIO_CHAN_INFO_SCALE: > > + *val = data->vref_mV; > > + *val2 = MCP47A1_MAX_STEP; > > [Severity: Medium] > Does this code use the correct denominator for the scale calculation? > > Since this is a 6-bit DAC, shouldn't the scale denominator be 64 (2^6) > instead of the maximum step value of 63? Using MCP47A1_MAX_STEP (63) here > introduces an off-by-one error that could cause userspace to calculate > output voltages with a ~1.5% LSB scaling error when reading the sysfs > attribute. Yes, the max amount of steps is 64, which should be passed here. I'll add a separate macro for this in v3. -- Kind regards, Joshua Crofts ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] iio: dac: mcp47a1: add support for new device 2026-07-28 13:15 ` Joshua Crofts @ 2026-08-01 2:30 ` Jonathan Cameron 2026-08-01 5:28 ` Joshua Crofts 0 siblings, 1 reply; 8+ messages in thread From: Jonathan Cameron @ 2026-08-01 2:30 UTC (permalink / raw) To: Joshua Crofts Cc: sashiko-bot, sashiko-reviews, devicetree, conor+dt, robh, linux-iio, andy, nuno.sa, dlechner On Tue, 28 Jul 2026 15:15:59 +0200 Joshua Crofts <joshua.crofts1@gmail.com> wrote: > On Mon, 27 Jul 2026 18:42:26 +0000 > sashiko-bot@kernel.org wrote: > > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > > - [Medium] Potential off-by-one in DAC scale calculation denominator. > > -- > > Responding to Sashiko's comment. > > > --- Patch [2]: [PATCH v2 2/2] iio: dac: mcp47a1: add support for new device --- > > commit 38bec2fbb020c3f749c52a26e5cc3e78e72eeaf5 > > Author: Joshua Crofts <joshua.crofts1@gmail.com> > > > > iio: dac: mcp47a1: add support for new device > > > > The Microchip MCP47A1 is a 6-bit volatile Digital-to-Analog converter > > which communicates via I2C. > > > > > diff --git a/drivers/iio/dac/mcp47a1.c b/drivers/iio/dac/mcp47a1.c > > > new file mode 100644 > > > index 0000000000000..140e93ff2ba01 > > > --- /dev/null > > > +++ b/drivers/iio/dac/mcp47a1.c > > [ ... ] > > > +static int mcp47a1_read(struct iio_dev *indio_dev, > > > + struct iio_chan_spec const *chan, > > > + int *val, int *val2, long mask) > > > +{ > > > + struct mcp47a1_data *data = iio_priv(indio_dev); > > > + int ret; > > > + > > > + switch (mask) { > > [ ... ] > > > + case IIO_CHAN_INFO_SCALE: > > > + *val = data->vref_mV; > > > + *val2 = MCP47A1_MAX_STEP; > > > > [Severity: Medium] > > Does this code use the correct denominator for the scale calculation? > > > > Since this is a 6-bit DAC, shouldn't the scale denominator be 64 (2^6) > > instead of the maximum step value of 63? Using MCP47A1_MAX_STEP (63) here > > introduces an off-by-one error that could cause userspace to calculate > > output voltages with a ~1.5% LSB scaling error when reading the sysfs > > attribute. > > Yes, the max amount of steps is 64, which should be passed here. I'll > add a separate macro for this in v3. I'd just use + 1 at this location given the clear relationship between the maximum value that can be taken and the number of steps. > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] iio: dac: mcp47a1: add support for new device 2026-08-01 2:30 ` Jonathan Cameron @ 2026-08-01 5:28 ` Joshua Crofts 0 siblings, 0 replies; 8+ messages in thread From: Joshua Crofts @ 2026-08-01 5:28 UTC (permalink / raw) To: Jonathan Cameron Cc: sashiko-bot, sashiko-reviews, devicetree, conor+dt, robh, linux-iio, andy, nuno.sa, dlechner On Sat, 1 Aug 2026 03:30:13 +0100 Jonathan Cameron <jic23@kernel.org> wrote: > On Tue, 28 Jul 2026 15:15:59 +0200 > Joshua Crofts <joshua.crofts1@gmail.com> wrote: > > > On Mon, 27 Jul 2026 18:42:26 +0000 > > sashiko-bot@kernel.org wrote: > > > > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > > > - [Medium] Potential off-by-one in DAC scale calculation denominator. > > > -- > > > > Responding to Sashiko's comment. > > > > > --- Patch [2]: [PATCH v2 2/2] iio: dac: mcp47a1: add support for new device --- > > > commit 38bec2fbb020c3f749c52a26e5cc3e78e72eeaf5 > > > Author: Joshua Crofts <joshua.crofts1@gmail.com> > > > > > > iio: dac: mcp47a1: add support for new device > > > > > > The Microchip MCP47A1 is a 6-bit volatile Digital-to-Analog converter > > > which communicates via I2C. > > > > > > > diff --git a/drivers/iio/dac/mcp47a1.c b/drivers/iio/dac/mcp47a1.c > > > > new file mode 100644 > > > > index 0000000000000..140e93ff2ba01 > > > > --- /dev/null > > > > +++ b/drivers/iio/dac/mcp47a1.c > > > [ ... ] > > > > +static int mcp47a1_read(struct iio_dev *indio_dev, > > > > + struct iio_chan_spec const *chan, > > > > + int *val, int *val2, long mask) > > > > +{ > > > > + struct mcp47a1_data *data = iio_priv(indio_dev); > > > > + int ret; > > > > + > > > > + switch (mask) { > > > [ ... ] > > > > + case IIO_CHAN_INFO_SCALE: > > > > + *val = data->vref_mV; > > > > + *val2 = MCP47A1_MAX_STEP; > > > > > > [Severity: Medium] > > > Does this code use the correct denominator for the scale calculation? > > > > > > Since this is a 6-bit DAC, shouldn't the scale denominator be 64 (2^6) > > > instead of the maximum step value of 63? Using MCP47A1_MAX_STEP (63) here > > > introduces an off-by-one error that could cause userspace to calculate > > > output voltages with a ~1.5% LSB scaling error when reading the sysfs > > > attribute. > > > > Yes, the max amount of steps is 64, which should be passed here. I'll > > add a separate macro for this in v3. > I'd just use + 1 at this location given the clear relationship between > the maximum value that can be taken and the number of steps. Good to know, for some reason I thought that this type of stuff was discouraged (MACRO_NAME + x would seem messy in most cases). -- Kind regards, Joshua Crofts ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] iio: dac: mcp47a1: add support for new device 2026-07-27 18:31 ` [PATCH v2 2/2] iio: dac: mcp47a1: add support for new device Joshua Crofts 2026-07-27 18:42 ` sashiko-bot @ 2026-08-01 2:31 ` Jonathan Cameron 1 sibling, 0 replies; 8+ messages in thread From: Jonathan Cameron @ 2026-08-01 2:31 UTC (permalink / raw) To: Joshua Crofts Cc: David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio, devicetree, linux-kernel > diff --git a/drivers/iio/dac/mcp47a1.c b/drivers/iio/dac/mcp47a1.c > new file mode 100644 > index 000000000000..140e93ff2ba0 > --- /dev/null > +++ b/drivers/iio/dac/mcp47a1.c > @@ -0,0 +1,169 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Microchip MCP47A1 DAC driver > + * > + * Copyright (c) 2026 Joshua Crofts <joshua.crofts1@gmail.com> > + * > + * Datasheet: https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/25154A.pdf > + */ > + > +#include <linux/array_size.h> > +#include <linux/delay.h> > +#include <linux/err.h> > +#include <linux/i2c.h> > +#include <linux/module.h> > +#include <linux/regulator/consumer.h> > +#include <linux/types.h> > +#include <linux/units.h> > + > +#include <linux/iio/iio.h> > + > +#define MCP47A1_REG_MAX 0x40 > +#define MCP47A1_CMD_CODE 0x00 > +#define MCP47A1_MAX_STEP 63 > + > +struct mcp47a1_data { > + struct i2c_client *client; > + int vref_mV; > +}; > + > +static const int mcp47a1_raw_avail[] = { 0, 1, MCP47A1_MAX_STEP }; > + > +static const struct iio_chan_spec mcp47a1_channels[] = { > + { > + .type = IIO_VOLTAGE, > + .indexed = 1, > + .output = 1, > + .channel = 0, > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > + .info_mask_separate_available = BIT(IIO_CHAN_INFO_RAW), > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), > + }, Given you are doing a v3 anyway, drop the array. A single channel is fine - just use &mc47a1_channel when referring to it and provide channel count of 1 directly (obviously correct as it is clear you aren't pointing to an array). > +}; ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-08-01 5:28 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-27 18:31 [PATCH v2 0/2] iio: dac: mcp47a1: add support for new device Joshua Crofts 2026-07-27 18:31 ` [PATCH v2 1/2] dt-bindings: iio: dac: add support for mcp47a1 Joshua Crofts 2026-07-27 18:31 ` [PATCH v2 2/2] iio: dac: mcp47a1: add support for new device Joshua Crofts 2026-07-27 18:42 ` sashiko-bot 2026-07-28 13:15 ` Joshua Crofts 2026-08-01 2:30 ` Jonathan Cameron 2026-08-01 5:28 ` Joshua Crofts 2026-08-01 2:31 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox