* Re: [PATCH 1/2] iio: dac: dac8163: Add driver for DAC8163
2026-06-23 16:07 ` [PATCH 1/2] iio: dac: dac8163: Add driver for DAC8163 Lukas Metz
@ 2026-06-23 16:18 ` sashiko-bot
2026-06-23 18:56 ` Siratul Islam
` (2 subsequent siblings)
3 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-06-23 16:18 UTC (permalink / raw)
To: Lukas Metz; +Cc: conor+dt, devicetree, robh
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The SPI transfer buffer `tx` is allocated on the stack, which is not DMA-safe and violates SPI subsystem requirements.
- [High] The pointer returned by `spi_get_device_match_data()` is unconditionally dereferenced without a NULL check.
- [Medium] The IIO scale attribute calculation ignores the hardware gain when the internal reference is used, returning an incorrect scale factor.
--
commit db89a9af22e26ad0341eb42f1659ed8a2fad0e51
Author: Lukas Metz <lukas.metz@gmx.net>
iio: dac: dac8163: Add driver for DAC8163
This commit adds a new driver for the Texas Instruments DAC756x, DAC816x,
and DAC856x series devices. These are low-power, voltage-output,
dual-channel DACs with up to 16 bits of resolution.
> diff --git a/drivers/iio/dac/ti-dac8163.c b/drivers/iio/dac/ti-dac8163.c
> new file mode 100644
> index 0000000000000..84a9dfb5347d6
> --- /dev/null
> +++ b/drivers/iio/dac/ti-dac8163.c
[ ... ]
> +static int dacxx6x_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int *val,
> + int *val2, long mask)
> +{
> + struct dacxx6x_state *st;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
[ ... ]
> + case IIO_CHAN_INFO_SCALE:
> + st = iio_priv(indio_dev);
> + *val = st->vref_uv / MILLI; /* vref in mV */
> + *val2 = chan->scan_type.realbits;
[Severity: Medium]
Does this scale calculation account for the hardware gain when using the
internal reference?
When st->internal_ref is true, the internal reference is 2.5V, but these
devices typically have a default internal gain of 2, making the full-scale
voltage 5V.
Should the gain multiplier be factored into the vref_uv calculation before
exposing the scale to userspace?
> + return IIO_VAL_FRACTIONAL_LOG2;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int dacxx6x_write_reg(struct dacxx6x_state *st, int reg, int addr,
> + unsigned int val)
> +{
> + u8 tx[3];
> +
> + tx[0] = COMMAND_SET(reg, addr);
> + tx[1] = (val >> 8) & 0xff;
> + tx[2] = val & 0xff;
> +
> + return spi_write(st->spi, tx, sizeof(tx));
[Severity: High]
Is it safe to pass a stack-allocated buffer to spi_write() here?
Since spi_write() maps the provided buffer for DMA, passing a stack
variable can lead to invalid physical address mappings or memory corruption,
especially on systems with CONFIG_VMAP_STACK enabled.
Could the tx buffer be moved into the dacxx6x_state structure, protected by
the lock, and properly cacheline-aligned to ensure it is DMA-safe?
> +}
[ ... ]
> +static int dacxx6x_probe(struct spi_device *spi)
> +{
[ ... ]
> + ret = dacxx6x_write_reg(st, CMD_SEL_REFERENCE, 0,
> + FIELD_PREP(VOLTAGE_REFERENCE_MASK, st->internal_ref));
> +
> + if (ret < 0)
> + goto err;
> +
> + info = spi_get_device_match_data(spi);
> +
> + indio_dev->name = info->name;
[Severity: High]
Can spi_get_device_match_data() return NULL here if the device is
instantiated from userspace via sysfs (using driver_override and bind)
with a name that doesn't match the ID tables?
If it does return NULL, the subsequent dereference of info->name will cause
a kernel oops. Is a NULL check needed before accessing info?
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->info = &dacxx6x_iio_info;
> + indio_dev->channels = info->channels;
> + indio_dev->num_channels = 2;
> +
> + ret = iio_device_register(indio_dev);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260623-dac8163-work-v1-0-5b508158faa0@gmx.net?part=1
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/2] iio: dac: dac8163: Add driver for DAC8163
2026-06-23 16:07 ` [PATCH 1/2] iio: dac: dac8163: Add driver for DAC8163 Lukas Metz
2026-06-23 16:18 ` sashiko-bot
@ 2026-06-23 18:56 ` Siratul Islam
2026-06-23 19:39 ` Andy Shevchenko
2026-06-23 19:50 ` David Lechner
3 siblings, 0 replies; 13+ messages in thread
From: Siratul Islam @ 2026-06-23 18:56 UTC (permalink / raw)
To: lukas.metz
Cc: andy, conor+dt, devicetree, dlechner, jic23, krzk+dt, linux-iio,
linux-kernel, nuno.sa, robh
On Tue, 2026-06-23 at 18:07 +0200, Lukas Metz wrote:
> The DAC756x, DAC816x, and DAC856x devices are low-power, voltage-output,
> dual-channel, 12-, 14-, and 16-bit digital-to-analog converters (DACs),
> respectively. These devices include a 2.5-V, 4-ppm/°C internal
> reference, giving a full-scale output voltage range of 2.5 V or 5 V.
>
> Signed-off-by: Lukas Metz <lukas.metz@gmx.net>
> ---
> MAINTAINERS | 6 +
> drivers/iio/dac/Kconfig | 10 ++
> drivers/iio/dac/Makefile | 1 +
> drivers/iio/dac/ti-dac8163.c | 339 +++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 356 insertions(+)
Hi! I took a quick look, and probably missed a lot of stuff. But here are my thoughts.
> diff --git a/MAINTAINERS b/MAINTAINERS
...
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * DACxx6x IIO driver (SPI)
> + */
A link to the datasheet here would be nice.
> +
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +#include <linux/of.h>
> +#include <linux/err.h>
> +#include <linux/slab.h>
> +#include <linux/iio/iio.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/units.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/printk.h>
> +#include <linux/bitfield.h>
Sort the includes alphabetically. And include what you use. "mod_devicetable.h" is missing for example.
While at it, separate the core headers from "<linux/iio/iio.h>". add ir in a sepatate line.
> +
> +#define COMMAND_MASK GENMASK(6, 3)
> +#define ADDRESS_MASK GENMASK(2, 0)
> +
> +#define COMMAND_SET(x, y) (FIELD_PREP(COMMAND_MASK, (x)) | \
> + FIELD_PREP(ADDRESS_MASK, (y)))
I'd align the FIELD_PREPs to make it look better. It may also fit in a single line.
> +
> +#define CMD_WRITE_INPUT_REG 0x0
> +#define CMD_UPDATE_DAC 0x1
> +#define CMD_WRITE_UPDATE_ALL 0x2
> +#define CMD_WRITE_UPDATE 0x3
> +#define CMD_SET_PWR_MODE 0x4
> +#define CMD_SOFT_RST 0x5
> +
> +#define CMD_LDAC_MODE 0x6
> +#define LDAC_MODE_CHANNEL_A_MASK BIT(0)
> +#define LDAC_MODE_CHANNEL_B_MASK BIT(1)
> +
> +#define CMD_SEL_REFERENCE 0x7
> +#define VOLTAGE_REFERENCE_MASK BIT(0)
> +
Group the CMD values together, also all these values would look better aligned.
> +enum dacxx6x_ldac_modes {
> + LDAC_MODE_ACTIVE = 0,
> + LDAC_MODE_INACTIVE = 1
> +};
A trailing comma would be nice.
> +
> +enum dacxx6x_voltage_reference {
> + VOLTAGE_REFERENCE_EXTERNAL = 0,
> + VOLTAGE_REFERENCE_INTERNAL = 1
> +};
Same here
> +
> +enum dacxx6x_supported_device_ids {
> + ID_DAC7562,
> + ID_DAC7563,
> + ID_DAC8162,
> + ID_DAC8163,
> + ID_DAC8562,
> + ID_DAC8563
> +};
> +
Here too.
>
> +struct dacxx6x_state {
Since the filename is dac8163.c, how about naming the functions/structs/other symbols that as well instead of dacxx6x?
> + struct spi_device *spi;
> +
How about use regmap?
> + struct regulator *vref;
> + struct gpio_desc *loaddacs;
> +
> + bool internal_ref;
> + int vref_uv;
> +
> + unsigned int cached[2];
> +
> + /*
> + * Lock to protect the state of the device from potential concurrent
> + * write accesses from userspace.
> + */
> + struct mutex lock;
> +};
> +
> +struct dacxx6x_chip_info {
> + const char *name;
> + const struct iio_chan_spec channels[2];
> +};
> +
> +#define DACXX6X_CHAN(id, resolution) \
> + { \
> + .type = IIO_VOLTAGE, .channel = (id), .output = 1, \
> + .indexed = 1, .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> + .scan_type = { .realbits = (resolution), \
> + .shift = 16 - (resolution) }, \
> + }
> +
> +static const struct dacxx6x_chip_info dacxx6x_chip_info_table[6] = {
> + [ID_DAC7562] = {
> + .name = "dac7562",
> + .channels = {
> + DACXX6X_CHAN(0, 12),
> + DACXX6X_CHAN(1, 12),
> + }
> + },
> + [ID_DAC7563] = {
> + .name = "dac7563",
> + .channels = {
> + DACXX6X_CHAN(0, 12),
> + DACXX6X_CHAN(1, 12),
> + }
> + },
> + [ID_DAC8162] = {
> + .name = "dac8162",
> + .channels = {
> + DACXX6X_CHAN(0, 14),
> + DACXX6X_CHAN(1, 14),
> + }
> + },
> + [ID_DAC8163] = {
> + .name = "dac8163",
> + .channels = {
> + DACXX6X_CHAN(0, 14),
> + DACXX6X_CHAN(1, 14),
> + }
> + },
> + [ID_DAC8562] = {
> + .name = "dac8562",
> + .channels = {
> + DACXX6X_CHAN(0, 16),
> + DACXX6X_CHAN(1, 16),
> + }
> + },
> + [ID_DAC8563] = {
> + .name = "dac8563",
> + .channels = {
> + DACXX6X_CHAN(0, 16),
> + DACXX6X_CHAN(1, 16),
> + }
> + },
> +};
> +
> +static int dacxx6x_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int *val,
> + int *val2, long mask)
> +{
> + struct dacxx6x_state *st;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + st = iio_priv(indio_dev);
Could this be assigned before the switch?
> + mutex_lock(&st->lock);
> + *val = st->cached[chan->channel];
> + mutex_unlock(&st->lock);
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
> + st = iio_priv(indio_dev);
> + *val = st->vref_uv / MILLI; /* vref in mV */
> + *val2 = chan->scan_type.realbits;
> + return IIO_VAL_FRACTIONAL_LOG2;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int dacxx6x_write_reg(struct dacxx6x_state *st, int reg, int addr,
> + unsigned int val)
> +{
> + u8 tx[3];
> +
> + tx[0] = COMMAND_SET(reg, addr);
> + tx[1] = (val >> 8) & 0xff;
How about put_unaligned_be16?
>
> + tx[2] = val & 0xff;
> +
> + return spi_write(st->spi, tx, sizeof(tx));
> +}
> +
> +static int dacxx6x_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int val,
> + int val2, long mask)
> +{
> + struct dacxx6x_state *st = iio_priv(indio_dev);
> + struct device *dev = &st->spi->dev;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + dev_dbg(dev, "%s: val=%d val2=%d\n", __func__, val, val2);
> + if (val2 != 0)
> + return -EINVAL;
> +
> + if (val < 0 || val >= BIT(chan->scan_type.realbits))
> + return -EINVAL;
> +
> + mutex_lock(&st->lock);
> + int ret = dacxx6x_write_reg(st, CMD_WRITE_UPDATE, chan->channel,
> + (unsigned int)val
> + << chan->scan_type.shift);
This case should be enclosed { }. Also, Use guard() from "cleanup.h" instead of manual mutex lock/unlock. Here and in
other places.
> +
> + if (!ret)
> + st->cached[chan->channel] = val;
> + mutex_unlock(&st->lock);
> + return ret;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static const struct iio_info dacxx6x_iio_info = {
> + .write_raw = dacxx6x_write_raw,
> + .read_raw = dacxx6x_read_raw
Trailing comma here
> +};
> +
> +static int dacxx6x_probe(struct spi_device *spi)
> +{
> + struct iio_dev *indio_dev;
> + struct dacxx6x_state *st;
> + const struct dacxx6x_chip_info *info;
> + int ret;
Sort these in a reverse christmas tree order.
> +
> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + st = iio_priv(indio_dev);
> + st->spi = spi;
> + spi_set_drvdata(spi, indio_dev);
> +
> + st->loaddacs = devm_gpiod_get_optional(&spi->dev, "ti,loaddacs",
> + GPIOD_OUT_LOW);
Vendor prefixes are not needed here.
> + if (IS_ERR(st->loaddacs))
> + return PTR_ERR(st->loaddacs);
> +
> + st->internal_ref =
> + device_property_read_bool(&spi->dev, "ti,internal-ref");
> +
> + if (!st->internal_ref) {
> + st->vref = devm_regulator_get(&spi->dev, "vref");
> + if (IS_ERR(st->vref))
> + return PTR_ERR(st->vref);
Maybe use return dev_err_probe?
> +
> + ret = regulator_enable(st->vref);
> + if (ret < 0)
> + return ret;
> + }
> +
> + mutex_init(&st->lock);
use devm_mutex_init.
> +
> + if (st->internal_ref) {
> + st->vref_uv = 2500000; /* 2.5V internal reference */
A note on where this value came from or why this was chosen, or a reference to datasheet would be better.
> + } else {
> + st->vref_uv = regulator_get_voltage(st->vref);
> + if (st->vref_uv < 0) {
> + ret = st->vref_uv;
> + goto err;
> + }
> + }
> +
You have a CMD_SOFT_RST defined but not used. Should this be used to reset before doing any configuration?
> + gpiod_set_value(st->loaddacs, 0);
> +
> + ret = dacxx6x_write_reg(st, CMD_LDAC_MODE, 0,
> + FIELD_PREP(LDAC_MODE_CHANNEL_A_MASK, LDAC_MODE_INACTIVE) |
> + FIELD_PREP(LDAC_MODE_CHANNEL_B_MASK, LDAC_MODE_INACTIVE));
> +
> + if (ret < 0)
> + goto err;
> +
> + ret = dacxx6x_write_reg(st, CMD_SEL_REFERENCE, 0,
> + FIELD_PREP(VOLTAGE_REFERENCE_MASK, st->internal_ref));
> +
> + if (ret < 0)
> + goto err;
> +
> + info = spi_get_device_match_data(spi);
> +
> + indio_dev->name = info->name;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->info = &dacxx6x_iio_info;
> + indio_dev->channels = info->channels;
> + indio_dev->num_channels = 2;
use ARRAY_SIZE(info->channels) and include linux/array_size.h
> +
> + ret = iio_device_register(indio_dev);
Use devm_iio_device_register
> + if (ret)
> + goto err;
> +
> + return 0;
> +
> +err:
> + if (!st->internal_ref)
> + regulator_disable(st->vref);
> + mutex_destroy(&st->lock);
> + return ret;
> +}
> +
> +static void dacxx6x_remove(struct spi_device *spi)
> +{
> + struct iio_dev *indio_dev = spi_get_drvdata(spi);
> + struct dacxx6x_state *st = iio_priv(indio_dev);
> +
> + iio_device_unregister(indio_dev);
Using devm would help here too. No need to unregister manually
> + mutex_destroy(&st->lock);
> + if (!st->internal_ref)
> + regulator_disable(st->vref);
> +}
> +
> +#define DACXX6X_COMPATIBLE(of_compatible, id) \
> + { \
> + .compatible = of_compatible, \
> + .data = &dacxx6x_chip_info_table[id] \
> + }
> +
> +static const struct of_device_id dacxx6x_of_match[] = {
> + DACXX6X_COMPATIBLE("ti,dac7562", ID_DAC7562),
> + DACXX6X_COMPATIBLE("ti,dac7563", ID_DAC7563),
> + DACXX6X_COMPATIBLE("ti,dac8162", ID_DAC8162),
> + DACXX6X_COMPATIBLE("ti,dac8163", ID_DAC8163),
> + DACXX6X_COMPATIBLE("ti,dac8562", ID_DAC8562),
> + DACXX6X_COMPATIBLE("ti,dac8563", ID_DAC8563),
> + {}
{} should have a space "{ }"
> +};
> +MODULE_DEVICE_TABLE(of, dacxx6x_of_match);
> +
> +static const struct spi_device_id dacxx6x_id_table[] = {
> + { "dac7562", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC7562] },
> + { "dac7563", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC7563] },
> + { "dac8162", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8162] },
> + { "dac8163", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8163] },
> + { "dac8562", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8562] },
> + { "dac8563", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8563] },
> + {}
Same here
> +};
> +
> +MODULE_DEVICE_TABLE(spi, dacxx6x_id_table);
> +
> +static struct spi_driver dacxx6x_driver = {
> + .driver = {
> + .name = "ti-dacxx6x",
Name doesn't need vendor prefix.
> + .of_match_table = dacxx6x_of_match,
> + },
> + .probe = dacxx6x_probe,
> + .remove = dacxx6x_remove,
> + .id_table = dacxx6x_id_table,
> +};
> +
No space here.
> +module_spi_driver(dacxx6x_driver);
> +
> +MODULE_AUTHOR("Lukas Metz <lukas.metz@gmx.net>");
> +MODULE_DESCRIPTION("Texas Instruments 12/14/16-bit 2-channel DAC driver");
> +MODULE_LICENSE("GPL");
Thanks
Sirat
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/2] iio: dac: dac8163: Add driver for DAC8163
2026-06-23 16:07 ` [PATCH 1/2] iio: dac: dac8163: Add driver for DAC8163 Lukas Metz
2026-06-23 16:18 ` sashiko-bot
2026-06-23 18:56 ` Siratul Islam
@ 2026-06-23 19:39 ` Andy Shevchenko
2026-06-23 19:50 ` David Lechner
3 siblings, 0 replies; 13+ messages in thread
From: Andy Shevchenko @ 2026-06-23 19:39 UTC (permalink / raw)
To: Lukas Metz
Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-kernel,
linux-iio, devicetree
On Tue, Jun 23, 2026 at 06:07:27PM +0200, Lukas Metz wrote:
> The DAC756x, DAC816x, and DAC856x devices are low-power, voltage-output,
> dual-channel, 12-, 14-, and 16-bit digital-to-analog converters (DACs),
> respectively. These devices include a 2.5-V, 4-ppm/°C internal
> reference, giving a full-scale output voltage range of 2.5 V or 5 V.
> +config TI_DAC8163
> + tristate "Texas Instruments 12/14/16-bit 2-channel DAC driver"
> + depends on SPI_MASTER
> + help
> + Driver for the Texas Instruments digital-to-analog converter
> + family dacxx6x compatible with the variants DAC7562,
> + DAC7563, DAC8162, DAC8163, DAC8562 and DAC8563.
This list doesn't scale. Put one part number per line as
- DAC7... (channel bits)
- DAC8... (...)
- ...
> + If compiled as a module, it will be called ti-dac8163.
...
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +#include <linux/of.h>
No, for this header in new drivers. Can't we use device and/or fwnode property
APIs instead?
> +#include <linux/err.h>
> +#include <linux/slab.h>
> +#include <linux/iio/iio.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/units.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/printk.h>
? Usually one wants dev_printk.h.
> +#include <linux/bitfield.h>
Keep the list sorted, and also follow the IWYU principle.
...
> +#define COMMAND_SET(x, y) (FIELD_PREP(COMMAND_MASK, (x)) | \
> + FIELD_PREP(ADDRESS_MASK, (y)))
This is ugly indentation. Compare to
#define COMMAND_SET(x, y) \
(FIELD_PREP(COMMAND_MASK, (x)) | FIELD_PREP(ADDRESS_MASK, (y)))
...
> +enum dacxx6x_ldac_modes {
> + LDAC_MODE_ACTIVE = 0,
> + LDAC_MODE_INACTIVE = 1
Leave trailing commas in the non-terminator entries here and there.
> +};
...
> +enum dacxx6x_supported_device_ids {
> + ID_DAC7562,
> + ID_DAC7563,
> + ID_DAC8162,
> + ID_DAC8163,
> + ID_DAC8562,
> + ID_DAC8563
> +};
This is solely used to make indexed array with chip_info. Instead kill this
enum and use separate structures.
...
> +struct dacxx6x_state {
> + struct spi_device *spi;
Why not regmap?
> + struct regulator *vref;
> + struct gpio_desc *loaddacs;
> +
> + bool internal_ref;
> + int vref_uv;
_uV
> + unsigned int cached[2];
> +
> + /*
> + * Lock to protect the state of the device from potential concurrent
> + * write accesses from userspace.
> + */
> + struct mutex lock;
> +};
...
> +#define DACXX6X_CHAN(id, resolution) \
> + { \
> + .type = IIO_VOLTAGE, .channel = (id), .output = 1, \
> + .indexed = 1, .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
Do not put two or more things on the same line, it's unreadable.
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> + .scan_type = { .realbits = (resolution), \
> + .shift = 16 - (resolution) }, \
Make these to be 4 lines.
> + }
...
> +static const struct dacxx6x_chip_info dacxx6x_chip_info_table[6] = {
Drop the number in the square brackets, let compiler do that job. But see
above, this has to be just 6 different data structures.
> +};
...
> +static int dacxx6x_write_reg(struct dacxx6x_state *st, int reg, int addr,
> + unsigned int val)
> +{
> + u8 tx[3];
Are you sure about this? How would it work with DMA?
> + tx[0] = COMMAND_SET(reg, addr);
> + tx[1] = (val >> 8) & 0xff;
> + tx[2] = val & 0xff;
Use proper unaligned setters: put_unaligned_be16() from linux/unaligned.h.
> + return spi_write(st->spi, tx, sizeof(tx));
> +}
...
> +static int dacxx6x_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int val,
> + int val2, long mask)
Split logically:
static int dacxx6x_write_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
int val, int val2, long mask)
> +{
> + struct dacxx6x_state *st = iio_priv(indio_dev);
> + struct device *dev = &st->spi->dev;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + dev_dbg(dev, "%s: val=%d val2=%d\n", __func__, val, val2);
No. Is it RFC? PoC? Or production-ready? If not the latter, come when it will
be production-ready.
> + if (val2 != 0)
> + return -EINVAL;
> + if (val < 0 || val >= BIT(chan->scan_type.realbits))
> + return -EINVAL;
Why not ERANGE or something like this?
> +
> + mutex_lock(&st->lock);
Why not guard()()?
> + int ret = dacxx6x_write_reg(st, CMD_WRITE_UPDATE, chan->channel,
> + (unsigned int)val
> + << chan->scan_type.shift);
Awful indentation. Please, check
> + if (!ret)
No, use regular pattern: Check for error first.
> + st->cached[chan->channel] = val;
> + mutex_unlock(&st->lock);
> + return ret;
> + default:
> + return -EINVAL;
> + }
> +}
...
> +static int dacxx6x_probe(struct spi_device *spi)
> +{
> + struct iio_dev *indio_dev;
> + struct dacxx6x_state *st;
> + const struct dacxx6x_chip_info *info;
> + int ret;
Reversed xmas tree order, please.
> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + st = iio_priv(indio_dev);
> + st->spi = spi;
> + spi_set_drvdata(spi, indio_dev);
Is it being used?
> + st->loaddacs = devm_gpiod_get_optional(&spi->dev, "ti,loaddacs",
> + GPIOD_OUT_LOW);
In the above you used temporary variable for struct device, why not here?
> + if (IS_ERR(st->loaddacs))
> + return PTR_ERR(st->loaddacs);
> +
> + st->internal_ref =
> + device_property_read_bool(&spi->dev, "ti,internal-ref");
And here (and it becomes a single line as well).
> + if (!st->internal_ref) {
> + st->vref = devm_regulator_get(&spi->dev, "vref");
> + if (IS_ERR(st->vref))
> + return PTR_ERR(st->vref);
> +
> + ret = regulator_enable(st->vref);
> + if (ret < 0)
> + return ret;
> + }
Can't you get just voltage and be done with it for now?
> + mutex_init(&st->lock);
devm_mutex_init.
> + if (st->internal_ref) {
> + st->vref_uv = 2500000; /* 2.5V internal reference */
> + } else {
> + st->vref_uv = regulator_get_voltage(st->vref);
> + if (st->vref_uv < 0) {
> + ret = st->vref_uv;
> + goto err;
> + }
> + }
> +
> + gpiod_set_value(st->loaddacs, 0);
> +
> + ret = dacxx6x_write_reg(st, CMD_LDAC_MODE, 0,
> + FIELD_PREP(LDAC_MODE_CHANNEL_A_MASK, LDAC_MODE_INACTIVE) |
> + FIELD_PREP(LDAC_MODE_CHANNEL_B_MASK, LDAC_MODE_INACTIVE));
> +
Stray blank line.
> + if (ret < 0)
> + goto err;
> +
> + ret = dacxx6x_write_reg(st, CMD_SEL_REFERENCE, 0,
> + FIELD_PREP(VOLTAGE_REFERENCE_MASK, st->internal_ref));
> +
Ditto.
> + if (ret < 0)
> + goto err;
> +
> + info = spi_get_device_match_data(spi);
We need to check for NULL here due to driver_override.
> + indio_dev->name = info->name;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->info = &dacxx6x_iio_info;
> + indio_dev->channels = info->channels;
> + indio_dev->num_channels = 2;
> +
> + ret = iio_device_register(indio_dev);
> + if (ret)
> + goto err;
> +
> + return 0;
> +err:
> + if (!st->internal_ref)
> + regulator_disable(st->vref);
> + mutex_destroy(&st->lock);
> + return ret;
> +}
...
> +static void dacxx6x_remove(struct spi_device *spi)
> +{
> + struct iio_dev *indio_dev = spi_get_drvdata(spi);
> + struct dacxx6x_state *st = iio_priv(indio_dev);
> +
> + iio_device_unregister(indio_dev);
> + mutex_destroy(&st->lock);
> + if (!st->internal_ref)
> + regulator_disable(st->vref);
> +}
Finish devm conversion and drop this function.
...
> +#define DACXX6X_COMPATIBLE(of_compatible, id) \
> + { \
> + .compatible = of_compatible, \
> + .data = &dacxx6x_chip_info_table[id] \
> + }
No, just use directly, so drop this macro.
> +static const struct of_device_id dacxx6x_of_match[] = {
> + DACXX6X_COMPATIBLE("ti,dac7562", ID_DAC7562),
> + DACXX6X_COMPATIBLE("ti,dac7563", ID_DAC7563),
> + DACXX6X_COMPATIBLE("ti,dac8162", ID_DAC8162),
> + DACXX6X_COMPATIBLE("ti,dac8163", ID_DAC8163),
> + DACXX6X_COMPATIBLE("ti,dac8562", ID_DAC8562),
> + DACXX6X_COMPATIBLE("ti,dac8563", ID_DAC8563),
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, dacxx6x_of_match);
> +
> +static const struct spi_device_id dacxx6x_id_table[] = {
> + { "dac7562", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC7562] },
> + { "dac7563", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC7563] },
> + { "dac8162", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8162] },
> + { "dac8163", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8163] },
> + { "dac8562", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8562] },
> + { "dac8563", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8563] },
> + {}
> +};
> +
Unneeded blank line.
> +MODULE_DEVICE_TABLE(spi, dacxx6x_id_table);
> +
> +static struct spi_driver dacxx6x_driver = {
> + .driver = {
> + .name = "ti-dacxx6x",
> + .of_match_table = dacxx6x_of_match,
> + },
> + .probe = dacxx6x_probe,
> + .remove = dacxx6x_remove,
> + .id_table = dacxx6x_id_table,
> +};
> +
Ditto.
> +module_spi_driver(dacxx6x_driver);
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/2] iio: dac: dac8163: Add driver for DAC8163
2026-06-23 16:07 ` [PATCH 1/2] iio: dac: dac8163: Add driver for DAC8163 Lukas Metz
` (2 preceding siblings ...)
2026-06-23 19:39 ` Andy Shevchenko
@ 2026-06-23 19:50 ` David Lechner
3 siblings, 0 replies; 13+ messages in thread
From: David Lechner @ 2026-06-23 19:50 UTC (permalink / raw)
To: Lukas Metz, Jonathan Cameron, Nuno Sá, Andy Shevchenko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-kernel, linux-iio, devicetree
On 6/23/26 11:07 AM, Lukas Metz wrote:
> The DAC756x, DAC816x, and DAC856x devices are low-power, voltage-output,
> dual-channel, 12-, 14-, and 16-bit digital-to-analog converters (DACs),
> respectively. These devices include a 2.5-V, 4-ppm/°C internal
> reference, giving a full-scale output voltage range of 2.5 V or 5 V.
Nice and simple driver. Mostly just needs better alignment with usual
IIO conventions.
>
> Signed-off-by: Lukas Metz <lukas.metz@gmx.net>
> ---
> MAINTAINERS | 6 +
> drivers/iio/dac/Kconfig | 10 ++
> drivers/iio/dac/Makefile | 1 +
> drivers/iio/dac/ti-dac8163.c | 339 +++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 356 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d238590a31f2..e82cc28e1bc3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -26394,6 +26394,12 @@ S: Odd Fixes
> F: drivers/clk/ti/
> F: include/linux/clk/ti.h
>
> +TI DAC8163 DAC DRIVER
> +M: Lukas Metz <lukas.metz@gmx.net>
> +L: linux-iio@vger.kernel.org
> +S: Maintained
> +F: drivers/iio/dac/ti-dac8163.c
> +
> TI DATA TRANSFORM AND HASHING ENGINE (DTHE) V2 CRYPTO DRIVER
> M: T Pratham <t-pratham@ti.com>
> L: linux-crypto@vger.kernel.org
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index db9f5c711b3d..6b6e5ee0732a 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -632,6 +632,16 @@ config TI_DAC7612
>
> If compiled as a module, it will be called ti-dac7612.
>
> +config TI_DAC8163
> + tristate "Texas Instruments 12/14/16-bit 2-channel DAC driver"
> + depends on SPI_MASTER
> + help
> + Driver for the Texas Instruments digital-to-analog converter
> + family dacxx6x compatible with the variants DAC7562,
> + DAC7563, DAC8162, DAC8163, DAC8562 and DAC8563.
> +
> + If compiled as a module, it will be called ti-dac8163.
> +
> config VF610_DAC
> tristate "Vybrid vf610 DAC driver"
> depends on HAS_IOMEM
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index 2a80bbf4e80a..359cde446623 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -62,4 +62,5 @@ obj-$(CONFIG_TI_DAC082S085) += ti-dac082s085.o
> obj-$(CONFIG_TI_DAC5571) += ti-dac5571.o
> obj-$(CONFIG_TI_DAC7311) += ti-dac7311.o
> obj-$(CONFIG_TI_DAC7612) += ti-dac7612.o
> +obj-$(CONFIG_TI_DAC8163) += ti-dac8163.o
> obj-$(CONFIG_VF610_DAC) += vf610_dac.o
> diff --git a/drivers/iio/dac/ti-dac8163.c b/drivers/iio/dac/ti-dac8163.c
> new file mode 100644
> index 000000000000..84a9dfb5347d
> --- /dev/null
> +++ b/drivers/iio/dac/ti-dac8163.c
> @@ -0,0 +1,339 @@
> +// SPDX-License-Identifier: GPL-2.0
Prefer GPL-2.0-only or GPL-2.0-or-later.
> +/*
> + * DACxx6x IIO driver (SPI)
> + */
> +
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +#include <linux/of.h>
> +#include <linux/err.h>
> +#include <linux/slab.h>
> +#include <linux/iio/iio.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/units.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/printk.h>
> +#include <linux/bitfield.h>
> +
> +#define COMMAND_MASK GENMASK(6, 3)
> +#define ADDRESS_MASK GENMASK(2, 0)
> +
> +#define COMMAND_SET(x, y) (FIELD_PREP(COMMAND_MASK, (x)) | \
> + FIELD_PREP(ADDRESS_MASK, (y)))
Usually, we try to avoid putting FIELD_PREP() in macros. This
is only used once in the code anyway, so dropping the macro
will actually save a line or two.
> +
> +#define CMD_WRITE_INPUT_REG 0x0
> +#define CMD_UPDATE_DAC 0x1
> +#define CMD_WRITE_UPDATE_ALL 0x2
> +#define CMD_WRITE_UPDATE 0x3
> +#define CMD_SET_PWR_MODE 0x4
> +#define CMD_SOFT_RST 0x5
> +
> +#define CMD_LDAC_MODE 0x6
> +#define LDAC_MODE_CHANNEL_A_MASK BIT(0)
> +#define LDAC_MODE_CHANNEL_B_MASK BIT(1)
> +
> +#define CMD_SEL_REFERENCE 0x7
> +#define VOLTAGE_REFERENCE_MASK BIT(0)
> +
> +enum dacxx6x_ldac_modes {
In IIO, we always avoid putting xx in identifier names. Just use dac8163
everwhere instead since that is the name of the driver.
> + LDAC_MODE_ACTIVE = 0,
> + LDAC_MODE_INACTIVE = 1
> +};
> +
> +enum dacxx6x_voltage_reference {
> + VOLTAGE_REFERENCE_EXTERNAL = 0,
> + VOLTAGE_REFERENCE_INTERNAL = 1
> +};
> +
> +enum dacxx6x_supported_device_ids {
> + ID_DAC7562,
> + ID_DAC7563,
> + ID_DAC8162,
> + ID_DAC8163,
> + ID_DAC8562,
> + ID_DAC8563
> +};
> +
> +struct dacxx6x_state {
> + struct spi_device *spi;
> +
> + struct regulator *vref;
This is
> + struct gpio_desc *loaddacs;
LDAC is very common in DACs, so I would call this ldac_gpio. Also, it isn't
currently used outside of probe, so we don't really need it here.
> +
> + bool internal_ref;
> + int vref_uv;
As in a later comment, we should be able to drop these as well if we can
get rid of the remove() callback.
> +
> + unsigned int cached[2];
Can we get a more descriptive name for this? Is it raw value?
> +
> + /*
> + * Lock to protect the state of the device from potential concurrent
> + * write accesses from userspace.
> + */
> + struct mutex lock;
> +};
> +
> +struct dacxx6x_chip_info {
> + const char *name;
> + const struct iio_chan_spec channels[2];
> +};
> +
> +#define DACXX6X_CHAN(id, resolution) \
> + { \
> + .type = IIO_VOLTAGE, .channel = (id), .output = 1, \
Please put each field on a new line.
> + .indexed = 1, .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> + .scan_type = { .realbits = (resolution), \
> + .shift = 16 - (resolution) }, \
> + }
> +
> +static const struct dacxx6x_chip_info dacxx6x_chip_info_table[6] = {
> + [ID_DAC7562] = {
> + .name = "dac7562",
> + .channels = {
> + DACXX6X_CHAN(0, 12),
> + DACXX6X_CHAN(1, 12),
> + }
> + },
> + [ID_DAC7563] = {
> + .name = "dac7563",
> + .channels = {
> + DACXX6X_CHAN(0, 12),
> + DACXX6X_CHAN(1, 12),
> + }
> + },
> + [ID_DAC8162] = {
> + .name = "dac8162",
> + .channels = {
> + DACXX6X_CHAN(0, 14),
> + DACXX6X_CHAN(1, 14),
> + }
> + },
> + [ID_DAC8163] = {
> + .name = "dac8163",
> + .channels = {
> + DACXX6X_CHAN(0, 14),
> + DACXX6X_CHAN(1, 14),
> + }
> + },
> + [ID_DAC8562] = {
> + .name = "dac8562",
> + .channels = {
> + DACXX6X_CHAN(0, 16),
> + DACXX6X_CHAN(1, 16),
> + }
> + },
> + [ID_DAC8563] = {
> + .name = "dac8563",
> + .channels = {
> + DACXX6X_CHAN(0, 16),
> + DACXX6X_CHAN(1, 16),
> + }
> + },
> +};
We are trying to get rid of arrays like this in drivers. We can just make
individual structs instead.
> +
> +static int dacxx6x_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int *val,
> + int *val2, long mask)
> +{
> + struct dacxx6x_state *st;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + st = iio_priv(indio_dev);
This can be moved out of the case statement so we don't have to
repeat it each time.
> + mutex_lock(&st->lock);
> + *val = st->cached[chan->channel];
> + mutex_unlock(&st->lock);
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
> + st = iio_priv(indio_dev);
> + *val = st->vref_uv / MILLI; /* vref in mV */
We've been writing this like:
*val = st->vref_uV / (MICRO/ MILLI);
Then we don't really need a comment.
> + *val2 = chan->scan_type.realbits;
> + return IIO_VAL_FRACTIONAL_LOG2;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int dacxx6x_write_reg(struct dacxx6x_state *st, int reg, int addr,
> + unsigned int val)
> +{
> + u8 tx[3];
> +
> + tx[0] = COMMAND_SET(reg, addr);
> + tx[1] = (val >> 8) & 0xff;
> + tx[2] = val & 0xff;
> +
> + return spi_write(st->spi, tx, sizeof(tx));
> +}
> +
> +static int dacxx6x_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int val,
> + int val2, long mask)
> +{
> + struct dacxx6x_state *st = iio_priv(indio_dev);
> + struct device *dev = &st->spi->dev;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + dev_dbg(dev, "%s: val=%d val2=%d\n", __func__, val, val2);
Do we really need to keep this debug print?
> + if (val2 != 0)
> + return -EINVAL;
> +
> + if (val < 0 || val >= BIT(chan->scan_type.realbits))
> + return -EINVAL;
> +
> + mutex_lock(&st->lock);
> + int ret = dacxx6x_write_reg(st, CMD_WRITE_UPDATE, chan->channel,
Usually, we would declare `int ret;` at the top of the function.
> + (unsigned int)val
Using u32 type instead of unsigned int would make this probably fit
on one line too.
> + << chan->scan_type.shift);
> +
> + if (!ret)
> + st->cached[chan->channel] = val;
> + mutex_unlock(&st->lock);
> + return ret;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static const struct iio_info dacxx6x_iio_info = {
> + .write_raw = dacxx6x_write_raw,
> + .read_raw = dacxx6x_read_raw
> +};
> +
> +static int dacxx6x_probe(struct spi_device *spi)
> +{
> + struct iio_dev *indio_dev;
> + struct dacxx6x_state *st;
> + const struct dacxx6x_chip_info *info;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + st = iio_priv(indio_dev);
> + st->spi = spi;
> + spi_set_drvdata(spi, indio_dev);
> +
> + st->loaddacs = devm_gpiod_get_optional(&spi->dev, "ti,loaddacs",
> + GPIOD_OUT_LOW);
As in the dt-bindings, we would expect the bindings to be active low
so this would be GPIOD_OUT_HIGH to assert the LDAC signal.
Could also use a comment to say that for now we are just holding this
asserted so that individual outputs are updated when we each raw attribute.
> + if (IS_ERR(st->loaddacs))
> + return PTR_ERR(st->loaddacs);
> +
> + st->internal_ref =
> + device_property_read_bool(&spi->dev, "ti,internal-ref");
> +
> + if (!st->internal_ref) {
> + st->vref = devm_regulator_get(&spi->dev, "vref");
> + if (IS_ERR(st->vref))
> + return PTR_ERR(st->vref);
> +
> + ret = regulator_enable(st->vref);
> + if (ret < 0)
> + return ret;
> + }
> +
> + mutex_init(&st->lock);
devm_mutex_init(). Also should not be in the middle of regulator code.
> +
> + if (st->internal_ref) {
> + st->vref_uv = 2500000; /* 2.5V internal reference */
> + } else {
> + st->vref_uv = regulator_get_voltage(st->vref);
> + if (st->vref_uv < 0) {
> + ret = st->vref_uv;
> + goto err;
> + }
> + }
The way we've been doing optional reference voltage lately is like this:
if (device_property_present(dev, "refin-supply")) {
ret = devm_regulator_get_enable_read_voltage(dev, "refin");
if (ret < 0)
return ret;
st->vref_mV = ret / (MICRO / MILLI);
} else {
st->vref_mV = DAC8163_INTERNAL_REF_mV;
}
This avoids the need for the ti,internal-ref DT property, avoid the need to convert
uV to mV later and the macro for the internal reference makes it self-documenting
so we don't need a comment. And it automacially cleans up after itself.
> +
> + gpiod_set_value(st->loaddacs, 0);
devm_gpiod_get_optional() already set this, so this is redundant.
> +
> + ret = dacxx6x_write_reg(st, CMD_LDAC_MODE, 0,
> + FIELD_PREP(LDAC_MODE_CHANNEL_A_MASK, LDAC_MODE_INACTIVE) |
> + FIELD_PREP(LDAC_MODE_CHANNEL_B_MASK, LDAC_MODE_INACTIVE));
> +
> + if (ret < 0)
> + goto err;
> +
> + ret = dacxx6x_write_reg(st, CMD_SEL_REFERENCE, 0,
> + FIELD_PREP(VOLTAGE_REFERENCE_MASK, st->internal_ref));
> +
Some of these lines are getting a bit long. We try to stick to close
to 80 columns in IIO when we can.
> + if (ret < 0)
> + goto err;
> +
> + info = spi_get_device_match_data(spi);
if (!info)
return -EINVAL;
> +
> + indio_dev->name = info->name;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->info = &dacxx6x_iio_info;
> + indio_dev->channels = info->channels;
> + indio_dev->num_channels = 2;
> +
> + ret = iio_device_register(indio_dev);
return devm_iio_device_register();
> + if (ret)
> + goto err;
> +
> + return 0;
> +
> +err:
> + if (!st->internal_ref)
> + regulator_disable(st->vref);
> + mutex_destroy(&st->lock);
> + return ret;
> +}
> +
> +static void dacxx6x_remove(struct spi_device *spi)
> +{
> + struct iio_dev *indio_dev = spi_get_drvdata(spi);
> + struct dacxx6x_state *st = iio_priv(indio_dev);
> +
> + iio_device_unregister(indio_dev);
> + mutex_destroy(&st->lock);
> + if (!st->internal_ref)
> + regulator_disable(st->vref);
We can use devm_* functions and avoid the need for the remove callback.
> +}
> +
> +#define DACXX6X_COMPATIBLE(of_compatible, id) \
> + { \
> + .compatible = of_compatible, \
> + .data = &dacxx6x_chip_info_table[id] \
> + }
> +
> +static const struct of_device_id dacxx6x_of_match[] = {
> + DACXX6X_COMPATIBLE("ti,dac7562", ID_DAC7562),
> + DACXX6X_COMPATIBLE("ti,dac7563", ID_DAC7563),
> + DACXX6X_COMPATIBLE("ti,dac8162", ID_DAC8162),
> + DACXX6X_COMPATIBLE("ti,dac8163", ID_DAC8163),
> + DACXX6X_COMPATIBLE("ti,dac8562", ID_DAC8562),
> + DACXX6X_COMPATIBLE("ti,dac8563", ID_DAC8563),
> + {}
IIO style is `{ }` (with space inbetween braces)
> +};
> +MODULE_DEVICE_TABLE(of, dacxx6x_of_match);
> +
> +static const struct spi_device_id dacxx6x_id_table[] = {
> + { "dac7562", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC7562] },
> + { "dac7563", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC7563] },
> + { "dac8162", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8162] },
> + { "dac8163", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8163] },
> + { "dac8562", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8562] },
> + { "dac8563", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8563] },
> + {}
> +};
> +
> +MODULE_DEVICE_TABLE(spi, dacxx6x_id_table);
> +
> +static struct spi_driver dacxx6x_driver = {
> + .driver = {
> + .name = "ti-dacxx6x",
> + .of_match_table = dacxx6x_of_match,
> + },
> + .probe = dacxx6x_probe,
> + .remove = dacxx6x_remove,
> + .id_table = dacxx6x_id_table,
> +};
> +
> +module_spi_driver(dacxx6x_driver);
> +
> +MODULE_AUTHOR("Lukas Metz <lukas.metz@gmx.net>");
> +MODULE_DESCRIPTION("Texas Instruments 12/14/16-bit 2-channel DAC driver");
> +MODULE_LICENSE("GPL");
>
^ permalink raw reply [flat|nested] 13+ messages in thread