Adds support for MCP48xx series of DACs.
Datasheet: https://ww1.microchip.com/downloads/en/DeviceDoc/22244B.pdf #MCP48x1
Datasheet: https://ww1.microchip.com/downloads/en/DeviceDoc/20002249B.pdf #MCP48x2
Reviewed-by: Conor Dooley <[email protected]>
Signed-off-by: Anshul Dalal <[email protected]>
---
Changes for v2:
- Changed order in device table to numerical
- Made vdd_supply required
- Added 'Reviewed-by: Conor Dooley'
Previous versions:
v1: https://lore.kernel.org/lkml/[email protected]/
---
.../bindings/iio/dac/microchip,mcp4821.yaml | 64 +++++++++++++++++++
1 file changed, 64 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/dac/microchip,mcp4821.yaml
diff --git a/Documentation/devicetree/bindings/iio/dac/microchip,mcp4821.yaml b/Documentation/devicetree/bindings/iio/dac/microchip,mcp4821.yaml
new file mode 100644
index 000000000000..97da9f9ef450
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/microchip,mcp4821.yaml
@@ -0,0 +1,64 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/dac/microchip,mcp4821.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip MCP4821 and similar DACs
+
+description: |
+ Supports MCP48x1 (single channel) and MCP48x2 (dual channel) series of DACs.
+ Device supports simplex communication over SPI in Mode 0,1 and Mode 1,1.
+
+ +---------+--------------+-------------+
+ | Device | Resolution | Channels |
+ |---------|--------------|-------------|
+ | MCP4801 | 8-bit | 1 |
+ | MCP4802 | 8-bit | 2 |
+ | MCP4811 | 10-bit | 1 |
+ | MCP4812 | 10-bit | 2 |
+ | MCP4821 | 12-bit | 1 |
+ | MCP4822 | 12-bit | 2 |
+ +---------+--------------+-------------+
+
+ Datasheet:
+ MCP48x1: https://ww1.microchip.com/downloads/en/DeviceDoc/22244B.pdf
+ MCP48x2: https://ww1.microchip.com/downloads/en/DeviceDoc/20002249B.pdf
+
+maintainers:
+ - Anshul Dalal <[email protected]>
+
+properties:
+ compatible:
+ enum:
+ - microchip,mcp4801
+ - microchip,mcp4802
+ - microchip,mcp4811
+ - microchip,mcp4812
+ - microchip,mcp4821
+ - microchip,mcp4822
+
+ reg:
+ maxItems: 1
+
+ vdd-supply: true
+
+required:
+ - compatible
+ - reg
+ - vdd-supply
+
+additionalProperties: false
+
+examples:
+ - |
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ dac@0 {
+ compatible = "microchip,mcp4821";
+ reg = <0>;
+ vdd-supply = <&vdd_regulator>;
+ };
+ };
--
2.43.0
Adds driver for the MCP48xx series of DACs.
Device uses a simplex SPI channel. To set the value of an output channel,
a 16-bit data of following format must be written:
Bit field | Description
15 [MSB] | Channel selection bit
0 -> Channel A
1 -> Channel B
13 | Output Gain Selection bit
0 -> 2x Gain (Vref = 4.096V)
1 -> 1x Gain (Vref = 2.048V)
12 | Output Shutdown Control bit
0 -> Shutdown the selected channel
1 -> Active mode operation
11-0 [LSB]| DAC Input Data bits
Value's big endian representation is taken as input for the
selected DAC channel. For devices with a resolution of less
than 12-bits, only the x most significant bits are considered
where x is the resolution of the device.
Reference: Page#22 [MCP48x2 Datasheet]
Supported devices:
+---------+--------------+-------------+
| Device | Resolution | Channels |
|---------|--------------|-------------|
| MCP4801 | 8-bit | 1 |
| MCP4802 | 8-bit | 2 |
| MCP4811 | 10-bit | 1 |
| MCP4812 | 10-bit | 2 |
| MCP4821 | 12-bit | 1 |
| MCP4822 | 12-bit | 2 |
+---------+--------------+-------------+
Devices tested:
MCP4821 [12-bit single channel]
MCP4802 [8-bit dual channel]
Tested on Raspberry Pi Zero 2W
Datasheet: https://ww1.microchip.com/downloads/en/DeviceDoc/22244B.pdf #MCP48x1
Datasheet: https://ww1.microchip.com/downloads/en/DeviceDoc/20002249B.pdf #MCP48x2
Signed-off-by: Anshul Dalal <[email protected]>
---
Changes for v2:
- Changed device ordering to numerical
- Fixed ordering of headers
- Added struct mcp4821_chip_info instead of relying on driver_data from
spi device ids
- Use scoped_guards for all mutex locks
- Changed write_val from __be16 to u16 in mcp4821_write_raw
Fixes sparse warning: incorrect type in assignment
Previous versions:
v1: https://lore.kernel.org/lkml/[email protected]/
---
MAINTAINERS | 7 ++
drivers/iio/dac/Kconfig | 10 ++
drivers/iio/dac/Makefile | 1 +
drivers/iio/dac/mcp4821.c | 228 ++++++++++++++++++++++++++++++++++++++
4 files changed, 246 insertions(+)
create mode 100644 drivers/iio/dac/mcp4821.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 81d5fc0bba68..8d9274c33c6e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13029,6 +13029,13 @@ F: Documentation/ABI/testing/sysfs-bus-iio-potentiometer-mcp4531
F: drivers/iio/potentiometer/mcp4018.c
F: drivers/iio/potentiometer/mcp4531.c
+MCP4821 DAC DRIVER
+M: Anshul Dalal <[email protected]>
+L: [email protected]
+S: Maintained
+F: Documentation/devicetree/bindings/iio/dac/microchip,mcp4821.yaml
+F: drivers/iio/dac/mcp4821.c
+
MCR20A IEEE-802.15.4 RADIO DRIVER
M: Stefan Schmidt <[email protected]>
L: [email protected]
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 93b8be183de6..34eb40bb9529 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -400,6 +400,16 @@ config MCP4728
To compile this driver as a module, choose M here: the module
will be called mcp4728.
+config MCP4821
+ tristate "MCP4801/02/11/12/21/22 DAC driver"
+ depends on SPI
+ help
+ Say yes here to build the driver for the Microchip MCP4801
+ MCP4802, MCP4811, MCP4812, MCP4821 and MCP4822 DAC devices.
+
+ To compile this driver as a module, choose M here: the module
+ will be called mcp4821.
+
config MCP4922
tristate "MCP4902, MCP4912, MCP4922 DAC driver"
depends on SPI
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 5b2bac900d5a..55bf89739d14 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -42,6 +42,7 @@ obj-$(CONFIG_MAX5522) += max5522.o
obj-$(CONFIG_MAX5821) += max5821.o
obj-$(CONFIG_MCP4725) += mcp4725.o
obj-$(CONFIG_MCP4728) += mcp4728.o
+obj-$(CONFIG_MCP4821) += mcp4821.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/mcp4821.c b/drivers/iio/dac/mcp4821.c
new file mode 100644
index 000000000000..7384df71f702
--- /dev/null
+++ b/drivers/iio/dac/mcp4821.c
@@ -0,0 +1,228 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2023 Anshul Dalal <[email protected]>
+ *
+ * Driver for Microchip MCP4801, MCP4802, MCP4811, MCP4812, MCP4821 and MCP4822
+ *
+ * Based on the work of:
+ * Michael Welling (MCP4922 Driver)
+ *
+ * Datasheet:
+ * MCP48x1: https://ww1.microchip.com/downloads/en/DeviceDoc/22244B.pdf
+ * MCP48x2: https://ww1.microchip.com/downloads/en/DeviceDoc/20002249B.pdf
+ *
+ * TODO:
+ * - Configurable gain
+ * - Regulator control
+ */
+
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/spi/spi.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/types.h>
+
+#include <asm/unaligned.h>
+
+#define MCP4821_ACTIVE_MODE BIT(12)
+#define MCP4802_SECOND_CHAN BIT(15)
+
+/* DAC uses an internal Voltage reference of 4.096V at a gain of 2x */
+#define MCP4821_2X_GAIN_VREF_MV 4096
+
+enum mcp4821_supported_drvice_ids {
+ ID_MCP4801,
+ ID_MCP4802,
+ ID_MCP4811,
+ ID_MCP4812,
+ ID_MCP4821,
+ ID_MCP4822,
+};
+
+struct mcp4821_state {
+ struct spi_device *spi;
+ // Protects dac_value
+ struct mutex lock;
+ u16 dac_value[2];
+};
+
+struct mcp4821_chip_info {
+ const char *name;
+ int num_channels;
+ const struct iio_chan_spec channels[2];
+};
+
+#define MCP4821_CHAN(channel_id, resolution) \
+ { \
+ .type = IIO_VOLTAGE, .output = 1, .indexed = 1, \
+ .channel = (channel_id), \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+ .scan_type = { \
+ .sign = 'u', \
+ .realbits = (resolution), \
+ .storagebits = 16, \
+ .shift = 12 - (resolution), \
+ }, \
+ }
+
+static const struct mcp4821_chip_info mcp4821_chip_info_table[6] = {
+ [ID_MCP4801] = {
+ .name = "mcp4801",
+ .num_channels = 1,
+ .channels = { MCP4821_CHAN(0, 8) }
+ },
+ [ID_MCP4802] = {
+ .name = "mcp4802",
+ .num_channels = 2,
+ .channels = { MCP4821_CHAN(0, 8),
+ MCP4821_CHAN(1, 8) }
+ },
+ [ID_MCP4811] = {
+ .name = "mcp4811",
+ .num_channels = 1,
+ .channels = { MCP4821_CHAN(0, 10) }
+ },
+ [ID_MCP4812] = {
+ .name = "mcp4812",
+ .num_channels = 2,
+ .channels = { MCP4821_CHAN(0, 10),
+ MCP4821_CHAN(1, 10) }
+ },
+ [ID_MCP4821] = {
+ .name = "mcp4821",
+ .num_channels = 1,
+ .channels = { MCP4821_CHAN(0, 12) }
+ },
+ [ID_MCP4822] = {
+ .name = "mcp4822",
+ .num_channels = 2,
+ .channels = { MCP4821_CHAN(0, 12),
+ MCP4821_CHAN(1, 12) }
+ },
+};
+
+static int mcp4821_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int *val,
+ int *val2, long mask)
+{
+ struct mcp4821_state *state;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ state = iio_priv(indio_dev);
+ scoped_guard(mutex, &state->lock)
+ *val = state->dac_value[chan->channel];
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ *val = MCP4821_2X_GAIN_VREF_MV;
+ *val2 = chan->scan_type.realbits;
+ return IIO_VAL_FRACTIONAL_LOG2;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int mcp4821_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int val,
+ int val2, long mask)
+{
+ struct mcp4821_state *state;
+ u16 write_val;
+ __be16 write_buffer;
+ int ret;
+ bool is_value_valid = val >= 0 && val < BIT(chan->scan_type.realbits) &&
+ val2 == 0;
+
+ if (!is_value_valid)
+ return -EINVAL;
+ if (mask != IIO_CHAN_INFO_RAW)
+ return -EINVAL;
+
+ state = iio_priv(indio_dev);
+ write_val = MCP4821_ACTIVE_MODE | val << chan->scan_type.shift;
+ if (chan->channel)
+ write_val |= MCP4802_SECOND_CHAN;
+ put_unaligned_be16(write_val, &write_buffer);
+ ret = spi_write(state->spi, &write_buffer, sizeof(write_buffer));
+ if (ret) {
+ dev_err(&state->spi->dev, "Failed to write to device: %d", ret);
+ return ret;
+ }
+
+ scoped_guard(mutex, &state->lock)
+ state->dac_value[chan->channel] = val;
+ return 0;
+}
+
+static const struct iio_info mcp4821_info = {
+ .read_raw = &mcp4821_read_raw,
+ .write_raw = &mcp4821_write_raw,
+};
+
+static int mcp4821_probe(struct spi_device *spi)
+{
+ struct iio_dev *indio_dev;
+ struct mcp4821_state *state;
+ const struct mcp4821_chip_info *info;
+
+ indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*state));
+ if (indio_dev == NULL)
+ return -ENOMEM;
+
+ state = iio_priv(indio_dev);
+ state->spi = spi;
+ mutex_init(&state->lock);
+
+ info = spi_get_device_match_data(spi);
+ indio_dev->name = info->name;
+ indio_dev->info = &mcp4821_info;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->channels = info->channels;
+ indio_dev->num_channels = info->num_channels;
+ return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+#define MCP4821_COMPATIBLE(of_compatible, id) \
+ { \
+ .compatible = of_compatible, \
+ .data = &mcp4821_chip_info_table[id] \
+ }
+
+static const struct of_device_id mcp4821_of_table[] = {
+ MCP4821_COMPATIBLE("microchip,mcp4801", ID_MCP4801),
+ MCP4821_COMPATIBLE("microchip,mcp4802", ID_MCP4802),
+ MCP4821_COMPATIBLE("microchip,mcp4811", ID_MCP4811),
+ MCP4821_COMPATIBLE("microchip,mcp4812", ID_MCP4812),
+ MCP4821_COMPATIBLE("microchip,mcp4821", ID_MCP4821),
+ MCP4821_COMPATIBLE("microchip,mcp4822", ID_MCP4822),
+ { /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mcp4821_of_table);
+
+static const struct spi_device_id mcp4821_id_table[] = {
+ { "mcp4801", (kernel_ulong_t)&mcp4821_chip_info_table[ID_MCP4801]},
+ { "mcp4802", (kernel_ulong_t)&mcp4821_chip_info_table[ID_MCP4802]},
+ { "mcp4811", (kernel_ulong_t)&mcp4821_chip_info_table[ID_MCP4811]},
+ { "mcp4812", (kernel_ulong_t)&mcp4821_chip_info_table[ID_MCP4812]},
+ { "mcp4821", (kernel_ulong_t)&mcp4821_chip_info_table[ID_MCP4821]},
+ { "mcp4822", (kernel_ulong_t)&mcp4821_chip_info_table[ID_MCP4822]},
+ { /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(spi, mcp4821_id_table);
+
+static struct spi_driver mcp4821_driver = {
+ .driver = {
+ .name = "mcp4821",
+ .of_match_table = mcp4821_of_table,
+ },
+ .probe = mcp4821_probe,
+ .id_table = mcp4821_id_table,
+};
+module_spi_driver(mcp4821_driver);
+
+MODULE_AUTHOR("Anshul Dalal <[email protected]>");
+MODULE_DESCRIPTION("Microchip MCP4821 DAC Driver");
+MODULE_LICENSE("GPL");
--
2.43.0
On Sun, Dec 17, 2023 at 12:11 PM Anshul Dalal <[email protected]> wrote:
>
> Adds support for MCP48xx series of DACs.
>
> Datasheet: https://ww1.microchip.com/downloads/en/DeviceDoc/22244B.pdf #MCP48x1
> Datasheet: https://ww1.microchip.com/downloads/en/DeviceDoc/20002249B.pdf #MCP48x2
> Reviewed-by: Conor Dooley <[email protected]>
> Signed-off-by: Anshul Dalal <[email protected]>
>
> ---
>
> Changes for v2:
> - Changed order in device table to numerical
> - Made vdd_supply required
> - Added 'Reviewed-by: Conor Dooley'
>
> Previous versions:
> v1: https://lore.kernel.org/lkml/[email protected]/
> ---
> .../bindings/iio/dac/microchip,mcp4821.yaml | 64 +++++++++++++++++++
> 1 file changed, 64 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/dac/microchip,mcp4821.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/dac/microchip,mcp4821.yaml b/Documentation/devicetree/bindings/iio/dac/microchip,mcp4821.yaml
> new file mode 100644
> index 000000000000..97da9f9ef450
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/microchip,mcp4821.yaml
> @@ -0,0 +1,64 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/dac/microchip,mcp4821.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip MCP4821 and similar DACs
> +
> +description: |
> + Supports MCP48x1 (single channel) and MCP48x2 (dual channel) series of DACs.
> + Device supports simplex communication over SPI in Mode 0,1 and Mode 1,1.
It seems like SPI modes usually only have one number in them, i.e.
mode 1 and mode 3 in this case, I'm guessing.
> +
> + +---------+--------------+-------------+
> + | Device | Resolution | Channels |
> + |---------|--------------|-------------|
> + | MCP4801 | 8-bit | 1 |
> + | MCP4802 | 8-bit | 2 |
> + | MCP4811 | 10-bit | 1 |
> + | MCP4812 | 10-bit | 2 |
> + | MCP4821 | 12-bit | 1 |
> + | MCP4822 | 12-bit | 2 |
> + +---------+--------------+-------------+
> +
> + Datasheet:
> + MCP48x1: https://ww1.microchip.com/downloads/en/DeviceDoc/22244B.pdf
> + MCP48x2: https://ww1.microchip.com/downloads/en/DeviceDoc/20002249B.pdf
> +
> +maintainers:
> + - Anshul Dalal <[email protected]>
> +
> +properties:
> + compatible:
> + enum:
> + - microchip,mcp4801
> + - microchip,mcp4802
> + - microchip,mcp4811
> + - microchip,mcp4812
> + - microchip,mcp4821
> + - microchip,mcp4822
> +
> + reg:
> + maxItems: 1
> +
> + vdd-supply: true
What about the SHDN and LDAC pins? It seems like all should have an
ldac-gpios property and MCP48x1 should have a shdn-gpios property for
these.
> +
> +required:
> + - compatible
> + - reg
> + - vdd-supply
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + dac@0 {
> + compatible = "microchip,mcp4821";
> + reg = <0>;
> + vdd-supply = <&vdd_regulator>;
> + };
> + };
> --
> 2.43.0
>
>
On Sun, Dec 17, 2023 at 7:25 PM David Lechner <[email protected]> wrote:
>
> On Sun, Dec 17, 2023 at 12:11 PM Anshul Dalal <[email protected]> wrote:
> >
> > Adds support for MCP48xx series of DACs.
> >
> > Datasheet: https://ww1.microchip.com/downloads/en/DeviceDoc/22244B.pdf #MCP48x1
> > Datasheet: https://ww1.microchip.com/downloads/en/DeviceDoc/20002249B.pdf #MCP48x2
> > Reviewed-by: Conor Dooley <[email protected]>
> > Signed-off-by: Anshul Dalal <[email protected]>
> >
> > ---
> >
> > Changes for v2:
> > - Changed order in device table to numerical
> > - Made vdd_supply required
> > - Added 'Reviewed-by: Conor Dooley'
> >
> > Previous versions:
> > v1: https://lore.kernel.org/lkml/[email protected]/
> > ---
> > .../bindings/iio/dac/microchip,mcp4821.yaml | 64 +++++++++++++++++++
> > 1 file changed, 64 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/iio/dac/microchip,mcp4821.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/iio/dac/microchip,mcp4821.yaml b/Documentation/devicetree/bindings/iio/dac/microchip,mcp4821.yaml
> > new file mode 100644
> > index 000000000000..97da9f9ef450
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/dac/microchip,mcp4821.yaml
> > @@ -0,0 +1,64 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/dac/microchip,mcp4821.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Microchip MCP4821 and similar DACs
> > +
> > +description: |
> > + Supports MCP48x1 (single channel) and MCP48x2 (dual channel) series of DACs.
> > + Device supports simplex communication over SPI in Mode 0,1 and Mode 1,1.
>
> It seems like SPI modes usually only have one number in them, i.e.
> mode 1 and mode 3 in this case, I'm guessing.
Oh, and doesn't this mean that we need to include spi-cpha and
spi-cpol properties since they aren't implicit in the bindings anymore
since [1]?
[1]: https://lore.kernel.org/all/[email protected]/
>
> > +
> > + +---------+--------------+-------------+
> > + | Device | Resolution | Channels |
> > + |---------|--------------|-------------|
> > + | MCP4801 | 8-bit | 1 |
> > + | MCP4802 | 8-bit | 2 |
> > + | MCP4811 | 10-bit | 1 |
> > + | MCP4812 | 10-bit | 2 |
> > + | MCP4821 | 12-bit | 1 |
> > + | MCP4822 | 12-bit | 2 |
> > + +---------+--------------+-------------+
> > +
> > + Datasheet:
> > + MCP48x1: https://ww1.microchip.com/downloads/en/DeviceDoc/22244B.pdf
> > + MCP48x2: https://ww1.microchip.com/downloads/en/DeviceDoc/20002249B.pdf
> > +
> > +maintainers:
> > + - Anshul Dalal <[email protected]>
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - microchip,mcp4801
> > + - microchip,mcp4802
> > + - microchip,mcp4811
> > + - microchip,mcp4812
> > + - microchip,mcp4821
> > + - microchip,mcp4822
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + vdd-supply: true
>
> What about the SHDN and LDAC pins? It seems like all should have an
> ldac-gpios property and MCP48x1 should have a shdn-gpios property for
> these.
>
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - vdd-supply
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + spi {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + dac@0 {
> > + compatible = "microchip,mcp4821";
> > + reg = <0>;
> > + vdd-supply = <&vdd_regulator>;
> > + };
> > + };
> > --
> > 2.43.0
> >
> >
On Sun, 17 Dec 2023 23:38:34 +0530
Anshul Dalal <[email protected]> wrote:
> Adds driver for the MCP48xx series of DACs.
>
> Device uses a simplex SPI channel. To set the value of an output channel,
> a 16-bit data of following format must be written:
>
> Bit field | Description
> 15 [MSB] | Channel selection bit
> 0 -> Channel A
> 1 -> Channel B
> 13 | Output Gain Selection bit
> 0 -> 2x Gain (Vref = 4.096V)
> 1 -> 1x Gain (Vref = 2.048V)
> 12 | Output Shutdown Control bit
> 0 -> Shutdown the selected channel
> 1 -> Active mode operation
> 11-0 [LSB]| DAC Input Data bits
> Value's big endian representation is taken as input for the
> selected DAC channel. For devices with a resolution of less
> than 12-bits, only the x most significant bits are considered
> where x is the resolution of the device.
> Reference: Page#22 [MCP48x2 Datasheet]
>
> Supported devices:
> +---------+--------------+-------------+
> | Device | Resolution | Channels |
> |---------|--------------|-------------|
> | MCP4801 | 8-bit | 1 |
> | MCP4802 | 8-bit | 2 |
> | MCP4811 | 10-bit | 1 |
> | MCP4812 | 10-bit | 2 |
> | MCP4821 | 12-bit | 1 |
> | MCP4822 | 12-bit | 2 |
> +---------+--------------+-------------+
>
> Devices tested:
> MCP4821 [12-bit single channel]
> MCP4802 [8-bit dual channel]
>
> Tested on Raspberry Pi Zero 2W
>
> Datasheet: https://ww1.microchip.com/downloads/en/DeviceDoc/22244B.pdf #MCP48x1
> Datasheet: https://ww1.microchip.com/downloads/en/DeviceDoc/20002249B.pdf #MCP48x2
> Signed-off-by: Anshul Dalal <[email protected]>
>
>
Hi Anshul,
Various minor comments inline.
Thanks,
Jonathan
> diff --git a/drivers/iio/dac/mcp4821.c b/drivers/iio/dac/mcp4821.c
> new file mode 100644
> index 000000000000..7384df71f702
> --- /dev/null
> +++ b/drivers/iio/dac/mcp4821.c
> @@ -0,0 +1,228 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2023 Anshul Dalal <[email protected]>
> + *
> + * Driver for Microchip MCP4801, MCP4802, MCP4811, MCP4812, MCP4821 and MCP4822
> + *
> + * Based on the work of:
> + * Michael Welling (MCP4922 Driver)
> + *
> + * Datasheet:
> + * MCP48x1: https://ww1.microchip.com/downloads/en/DeviceDoc/22244B.pdf
> + * MCP48x2: https://ww1.microchip.com/downloads/en/DeviceDoc/20002249B.pdf
> + *
> + * TODO:
> + * - Configurable gain
> + * - Regulator control
> + */
> +
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/spi/spi.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/types.h>
> +
> +#include <asm/unaligned.h>
> +
> +#define MCP4821_ACTIVE_MODE BIT(12)
> +#define MCP4802_SECOND_CHAN BIT(15)
> +
> +/* DAC uses an internal Voltage reference of 4.096V at a gain of 2x */
> +#define MCP4821_2X_GAIN_VREF_MV 4096
> +
> +enum mcp4821_supported_drvice_ids {
> + ID_MCP4801,
> + ID_MCP4802,
> + ID_MCP4811,
> + ID_MCP4812,
> + ID_MCP4821,
> + ID_MCP4822,
> +};
> +
> +struct mcp4821_state {
> + struct spi_device *spi;
> + // Protects dac_value
/* Protects dac_value */
However I'm not clear why a u16 array needs protecting?
Whilst in theory the compiler might be annoying and do torn writes
(write it as 2 1 byte values) that's pretty unlikely and
we don't normally bother to protect against that.
> + struct mutex lock;
> + u16 dac_value[2];
> +};
> +
> +struct mcp4821_chip_info {
> + const char *name;
> + int num_channels;
> + const struct iio_chan_spec channels[2];
> +};
> +
> +#define MCP4821_CHAN(channel_id, resolution) \
> + { \
> + .type = IIO_VOLTAGE, .output = 1, .indexed = 1, \
> + .channel = (channel_id), \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> + .scan_type = { \
> + .sign = 'u', \
Not used so don't bother setting it until you add buffered support.
> + .realbits = (resolution), \
> + .storagebits = 16, \
Not used, so don't bother setting it.
> + .shift = 12 - (resolution), \
> + }, \
> + }
> +static int mcp4821_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int *val,
> + int *val2, long mask)
> +{
> + struct mcp4821_state *state;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + state = iio_priv(indio_dev);
> + scoped_guard(mutex, &state->lock)
> + *val = state->dac_value[chan->channel];
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
> + *val = MCP4821_2X_GAIN_VREF_MV;
> + *val2 = chan->scan_type.realbits;
> + return IIO_VAL_FRACTIONAL_LOG2;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int mcp4821_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int val,
> + int val2, long mask)
> +{
> + struct mcp4821_state *state;
> + u16 write_val;
> + __be16 write_buffer;
> + int ret;
> + bool is_value_valid = val >= 0 && val < BIT(chan->scan_type.realbits) &&
> + val2 == 0;
> +
> + if (!is_value_valid)
Check the opposite and drop the local variable that adds nothing.
if ((val < 0) || (val >= BIT(chan_type).realbits)) || (val2 != 0))
or split it up into checks on val2 in one check then on val in the other.
> + return -EINVAL;
> + if (mask != IIO_CHAN_INFO_RAW)
> + return -EINVAL;
> +
> + state = iio_priv(indio_dev);
I'd do this at point of local variable declaration above.
struct mcp4821_state *state = iio_priv(indio_dev);
> + write_val = MCP4821_ACTIVE_MODE | val << chan->scan_type.shift;
> + if (chan->channel)
> + write_val |= MCP4802_SECOND_CHAN;
> + put_unaligned_be16(write_val, &write_buffer);
It's aligned given it's a __be16
> + ret = spi_write(state->spi, &write_buffer, sizeof(write_buffer));
> + if (ret) {
> + dev_err(&state->spi->dev, "Failed to write to device: %d", ret);
> + return ret;
> + }
> +
> + scoped_guard(mutex, &state->lock)
> + state->dac_value[chan->channel] = val;
> + return 0;
> +}
> +
Jonathan