On 7/16/20 9:27 AM, Cristian Pop wrote:
Implementation for 1-bit ADC (comparator) and a 1-bit DAC (switch)
Very sneaky way of introducing a iio-gpio-proxy driver to be able to
access GPIOs through libiio ;). I'm not really a fan of the whole idea.
But either way I think this needs a better description of what 1-bit
converters are and how they are used.
Signed-off-by: Cristian Pop <cristian.pop@xxxxxxxxxx>
---
drivers/iio/addac/one-bit-adc-dac.c | 229 ++++++++++++++++++++++++++++
1 file changed, 229 insertions(+)
create mode 100644 drivers/iio/addac/one-bit-adc-dac.c
diff --git a/drivers/iio/addac/one-bit-adc-dac.c b/drivers/iio/addac/one-bit-adc-dac.c
new file mode 100644
index 000000000000..8e2a8a09fedb
--- /dev/null
+++ b/drivers/iio/addac/one-bit-adc-dac.c
@@ -0,0 +1,229 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Analog Devices ONE_BIT_ADC_DAC
+ * Digital to Analog Converters driver
+ *
+ * Copyright 2019 Analog Devices Inc.
+ */
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/iio/iio.h>
+#include <linux/platform_device.h>
+#include <linux/gpio/consumer.h>
+
+enum ch_direction {
+ CH_IN,
+ CH_OUT,
+};
+
+struct one_bit_adc_dac_state {
+ struct platform_device *pdev;
+ struct gpio_descs *in_gpio_descs;
+ struct gpio_descs *out_gpio_descs;
+};
+
+ #define ONE_BIT_ADC_DAC_CHANNEL(idx, direction) \
+ { \
+ .type = IIO_VOLTAGE, \
+ .indexed = 1, \
+ .channel = idx, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .output = direction, \
+ }
+
+static int one_bit_adc_dac_read_raw(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan, int *val, int *val2, long info)
+{
+ struct one_bit_adc_dac_state *st = iio_priv(indio_dev);
+ int in_num_ch = 0, out_num_ch = 0;
+ int channel = chan->channel;
+
+ if (st->in_gpio_descs)
+ in_num_ch = st->in_gpio_descs->ndescs;
+
+ if (st->out_gpio_descs)
+ out_num_ch = st->out_gpio_descs->ndescs;
+
+ switch (info) {
+ case IIO_CHAN_INFO_RAW:
+ if (channel < in_num_ch) {
+ *val = gpiod_get_value_cansleep(
+ st->in_gpio_descs->desc[channel]);
+ } else {
+ channel -= in_num_ch;
+ *val = gpiod_get_value_cansleep(
+ st->out_gpio_descs->desc[channel]);
+ }
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int one_bit_adc_dac_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val,
+ int val2,
+ long info)
+{
+ struct one_bit_adc_dac_state *st = iio_priv(indio_dev);
+ int in_num_ch = 0, out_num_ch = 0;
+ int channel = chan->channel;
+
+ if (st->in_gpio_descs)
+ in_num_ch = st->in_gpio_descs->ndescs;
+
+ if (st->out_gpio_descs)
+ out_num_ch = st->out_gpio_descs->ndescs;
+
+ switch (info) {
+ case IIO_CHAN_INFO_RAW:
+ if (channel < in_num_ch) {
+ gpiod_set_value_cansleep(
+ st->in_gpio_descs->desc[channel], val);
How can we set a value on an input GPIO?
+ } else {
+ channel -= in_num_ch;
+ gpiod_set_value_cansleep(
+ st->out_gpio_descs->desc[channel], val);
+ }
+
+ return 0;
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct iio_info one_bit_adc_dac_info = {
+ .read_raw = &one_bit_adc_dac_read_raw,
+ .write_raw = &one_bit_adc_dac_write_raw,
+};
+
+static int one_bit_adc_dac_set_ch(struct iio_dev *indio_dev,
+ struct iio_chan_spec *channels,
+ const char *propname,
+ int num_ch,
+ enum ch_direction direction,
+ int offset)
+{
+ struct one_bit_adc_dac_state *st = iio_priv(indio_dev);
+ const char **gpio_names;
+ int ret, i;
+
+ if (num_ch <= 0)
+ return 0;
+
+ gpio_names = devm_kcalloc(indio_dev->dev.parent,
+ num_ch,
+ sizeof(char *),
sizeof(*gpio_names). It might be better to use normal kcalloc, kfree
here since you only use it in this function.
+ GFP_KERNEL);
+ if (!gpio_names)
+ return -ENOMEM;
+
+ ret = device_property_read_string_array(&st->pdev->dev,
+ propname,
+ gpio_names,
+ num_ch);
+ if (ret < 0)
+ return ret;
+
+ for (i = 0; i < num_ch; i++) {
+ channels[i] = (struct iio_chan_spec)ONE_BIT_ADC_DAC_CHANNEL(i +
+ offset,
+ direction);
+ channels[i].extend_name = gpio_names[i];
I think we want to avoid using extend_name in new drivers because it
makes for a very clumsy ABI. We should add a label property like we have
for the device for channels to have a symbolic name of the channel.
+ }
+
+ return 0;
+}
+
+static int one_bit_adc_dac_parse_dt(struct iio_dev *indio_dev)
+{
+ struct one_bit_adc_dac_state *st = iio_priv(indio_dev);
+ struct iio_chan_spec *channels;
+ int ret, in_num_ch = 0, out_num_ch = 0;
+
+ st->in_gpio_descs = devm_gpiod_get_array_optional(&st->pdev->dev,
+ "in", GPIOD_IN);
+ if (IS_ERR(st->in_gpio_descs))
+ return PTR_ERR(st->in_gpio_descs);
+
+ if (st->in_gpio_descs)
+ in_num_ch = st->in_gpio_descs->ndescs;
+
+ st->out_gpio_descs = devm_gpiod_get_array_optional(&st->pdev->dev,
+ "out", GPIOD_OUT_HIGH);
+ if (IS_ERR(st->out_gpio_descs))
+ return PTR_ERR(st->out_gpio_descs);
+
+ if (st->out_gpio_descs)
+ out_num_ch = st->out_gpio_descs->ndescs;
+
+ channels = devm_kcalloc(indio_dev->dev.parent, (in_num_ch + out_num_ch),
+ sizeof(struct iio_chan_spec), GFP_KERNEL);
sizeof(*channels) to avoid accidentally using the wrong type.
+ if (!channels)
+ return -ENOMEM;
+
+ ret = one_bit_adc_dac_set_ch(indio_dev, &channels[0],
+ "in-gpio-names", in_num_ch,
+ CH_IN, 0);
+ if (ret < 0)
+ return ret;
+
+ ret = one_bit_adc_dac_set_ch(indio_dev, &channels[in_num_ch],
+ "out-gpio-names", out_num_ch,
+ CH_OUT, in_num_ch);
+ if (ret < 0)
+ return ret;
+
+ indio_dev->channels = channels;
+ indio_dev->num_channels = in_num_ch + out_num_ch;
+
+ return 0;
+}
+
+static int one_bit_adc_dac_probe(struct platform_device *pdev)
+{
+ struct iio_dev *indio_dev;
+ struct one_bit_adc_dac_state *st;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*st));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ st = iio_priv(indio_dev);
+ st->pdev = pdev;
+ indio_dev->dev.parent = &pdev->dev;
parent assignment should not be needed thanks to Alex's work.
+ indio_dev->name = "one-bit-adc-dac";
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->info = &one_bit_adc_dac_info;
+
+ ret = one_bit_adc_dac_parse_dt(indio_dev);
+ if (ret < 0)
+ return ret;
+
+ platform_set_drvdata(pdev, indio_dev);
There does not seem to be a matching get_drvdata() anywhere so this is
not needed.
+ return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
+}
+
+static const struct of_device_id one_bit_adc_dac_dt_match[] = {
+ { .compatible = "adi,one-bit-adc-dac" },
+ {},
+};
+
+MODULE_DEVICE_TABLE(of, one_bit_adc_dac_dt_match);
+
+static struct platform_driver one_bit_adc_dac_driver = {
+ .driver = {
+ .name = "one-bit-adc-dac",
+ .of_match_table = one_bit_adc_dac_dt_match,
+ },
+ .probe = one_bit_adc_dac_probe,
+};
+
+module_platform_driver(one_bit_adc_dac_driver);
+
+MODULE_AUTHOR("Cristian Pop <cristian.pop@xxxxxxxxxx>");
+MODULE_DESCRIPTION("Analog Devices ONE_BIT_ADC_DAC");
+MODULE_LICENSE("GPL v2");
