[PATCH v6 0/3] iio: Add Cosmic Circuits ADC support

[PATCH v6 0/3] iio: Add Cosmic Circuits ADC support

[Ezequiel Garcia]

This series add the support for an ADC IP block from Cosmic Circuits.
The patchset is based on v3.19-rc3.

Rob/Mark: Jonathan has asked for an ack of the devicetree binding. Do you
think you can take a look to that?

Thanks a lot!
--
Changes from v5:

  * Fixed CC10001_ADC_CH_MASK macro definition, to separate the channel
    number from the channel map, as noted by Hartmut.
  * Return the real error code in devm_regulator_get failure.

Changes from v4:

  * Added a compile-time dependency on REGULATOR and HAVE_CLK.
  * Replaced the silly XOR operation for a proper mask out of the
    available channels.

Changes from v3:

  * Fixed a few style nitpicks as per Hartmut's feedback.
  * Used GENMASK() to build the channel mask, which fixes a very nasty
    bug. Also found by Hartmut.

Changes from v2:

  * Changed a devicetree property from adc-available-channels to
    adc-reserved-channels, so it can be made optional.
  * Renamed the driver from cc_10001_xxx to cc10001_xxx so it's consistent
    with the rest of the kernel style.
  * Some more minor cosmetic fixes.

Changes from v1:

  * Removed unneeded header includes.
  * Changed all the names and macros prefix: s/CC_10001_/CC10001_.
  * Used .update_scan_mode callback to preallocate the buffer.
  * Used indio_dev for the struct iio_dev.
  * Only read the regulator voltage when needed.
  * Fixed probe() error handling.
  * Used for_each_set_bit() instead of open-coding it.
  * Name the power-down register as _POWER_UP, to make the code
    less silly.
  * Error out when no valid sample can be read (i.e. when end-of-conversion
    poll times out).
  * ... plus some assorted code cleaning based on the feedback.

Ezequiel Garcia (1):
  DT: Add a vendor prefix for Cosmic Circuits

Phani Movva (2):
  iio: adc: Cosmic Circuits 10001 ADC driver
  DT: iio: adc: Add CC_10001 binding documentation

 .../devicetree/bindings/iio/adc/cc10001_adc.txt    |  22 ++
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 drivers/iio/adc/Kconfig                            |  11 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/cc10001_adc.c                      | 424 +++++++++++++++++++++
 5 files changed, 459 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt
 create mode 100644 drivers/iio/adc/cc10001_adc.c

-- 
2.2.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v6 1/3] iio: adc: Cosmic Circuits 10001 ADC driver

[Ezequiel Garcia]

From: Phani Movva <Phani.Movva-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>

This commit adds support for Cosmic Circuits 10001 10-bit ADC device.

Signed-off-by: Phani Movva <Phani.Movva-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Naidu Tellapati <Naidu.Tellapati-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
[ezequiel: code style cleaning]
Signed-off-by: Ezequiel Garcia <ezequiel.garcia-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
---
 drivers/iio/adc/Kconfig       |  11 ++
 drivers/iio/adc/Makefile      |   1 +
 drivers/iio/adc/cc10001_adc.c | 424 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 436 insertions(+)
 create mode 100644 drivers/iio/adc/cc10001_adc.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 0f79e47..9cb8543 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -135,6 +135,17 @@ config AXP288_ADC
 	  device. Depending on platform configuration, this general purpose ADC can
 	  be used for sampling sensors such as thermal resistors.
 
+config CC10001_ADC
+	tristate "Cosmic Circuits 10001 ADC driver"
+	depends on HAS_IOMEM || HAVE_CLK || REGULATOR
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+	help
+	  Say yes here to build support for Cosmic Circuits 10001 ADC.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called cc10001_adc.
+
 config EXYNOS_ADC
 	tristate "Exynos ADC driver support"
 	depends on ARCH_EXYNOS || ARCH_S3C24XX || ARCH_S3C64XX || (OF && COMPILE_TEST)
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 701fdb7..ce60fe9 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_AD7887) += ad7887.o
 obj-$(CONFIG_AD799X) += ad799x.o
 obj-$(CONFIG_AT91_ADC) += at91_adc.o
 obj-$(CONFIG_AXP288_ADC) += axp288_adc.o
+obj-$(CONFIG_CC10001_ADC) += cc10001_adc.o
 obj-$(CONFIG_EXYNOS_ADC) += exynos_adc.o
 obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
 obj-$(CONFIG_MAX1027) += max1027.o
diff --git a/drivers/iio/adc/cc10001_adc.c b/drivers/iio/adc/cc10001_adc.c
new file mode 100644
index 0000000..1ef4329
--- /dev/null
+++ b/drivers/iio/adc/cc10001_adc.c
@@ -0,0 +1,424 @@
+/*
+ * Copyright (c) 2014-2015 Imagination Technologies Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+#include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+
+/* Registers */
+#define CC10001_ADC_CONFIG		0x00
+#define CC10001_ADC_START_CONV		BIT(4)
+#define CC10001_ADC_MODE_SINGLE_CONV	BIT(5)
+
+#define CC10001_ADC_DDATA_OUT		0x04
+#define CC10001_ADC_EOC			0x08
+#define CC10001_ADC_EOC_SET		BIT(0)
+
+#define CC10001_ADC_CHSEL_SAMPLED	0x0c
+#define CC10001_ADC_POWER_UP		0x10
+#define CC10001_ADC_POWER_UP_SET	BIT(0)
+#define CC10001_ADC_DEBUG		0x14
+#define CC10001_ADC_DATA_COUNT		0x20
+
+#define CC10001_ADC_DATA_MASK		GENMASK(9, 0)
+#define CC10001_ADC_NUM_CHANNELS	8
+#define CC10001_ADC_CH_MASK		GENMASK(2, 0)
+
+#define CC10001_INVALID_SAMPLED		0xffff
+#define CC10001_MAX_POLL_COUNT		20
+
+/*
+ * As per device specification, wait six clock cycles after power-up to
+ * activate START. Since adding two more clock cycles delay does not
+ * impact the performance too much, we are adding two additional cycles delay
+ * intentionally here.
+ */
+#define	CC10001_WAIT_CYCLES		8
+
+struct cc10001_adc_device {
+	void __iomem *reg_base;
+	struct clk *adc_clk;
+	struct regulator *reg;
+	u16 *buf;
+
+	struct mutex lock;
+	unsigned long channel_map;
+	unsigned int start_delay_ns;
+	unsigned int eoc_delay_ns;
+};
+
+static inline void cc10001_adc_write_reg(struct cc10001_adc_device *adc_dev,
+					 u32 reg, u32 val)
+{
+	writel(val, adc_dev->reg_base + reg);
+}
+
+static inline u32 cc10001_adc_read_reg(struct cc10001_adc_device *adc_dev,
+				       u32 reg)
+{
+	return readl(adc_dev->reg_base + reg);
+}
+
+static void cc10001_adc_start(struct cc10001_adc_device *adc_dev,
+			      unsigned int channel)
+{
+	u32 val;
+
+	/* Channel selection and mode of operation */
+	val = (channel & CC10001_ADC_CH_MASK) | CC10001_ADC_MODE_SINGLE_CONV;
+	cc10001_adc_write_reg(adc_dev, CC10001_ADC_CONFIG, val);
+
+	val = cc10001_adc_read_reg(adc_dev, CC10001_ADC_CONFIG);
+	val = val | CC10001_ADC_START_CONV;
+	cc10001_adc_write_reg(adc_dev, CC10001_ADC_CONFIG, val);
+}
+
+static u16 cc10001_adc_poll_done(struct iio_dev *indio_dev,
+				 unsigned int channel,
+				 unsigned int delay)
+{
+	struct cc10001_adc_device *adc_dev = iio_priv(indio_dev);
+	unsigned int poll_count = 0;
+
+	while (!(cc10001_adc_read_reg(adc_dev, CC10001_ADC_EOC) &
+			CC10001_ADC_EOC_SET)) {
+
+		ndelay(delay);
+		if (poll_count++ == CC10001_MAX_POLL_COUNT)
+			return CC10001_INVALID_SAMPLED;
+	}
+
+	poll_count = 0;
+	while ((cc10001_adc_read_reg(adc_dev, CC10001_ADC_CHSEL_SAMPLED) &
+			CC10001_ADC_CH_MASK) != channel) {
+
+		ndelay(delay);
+		if (poll_count++ == CC10001_MAX_POLL_COUNT)
+			return CC10001_INVALID_SAMPLED;
+	}
+
+	/* Read the 10 bit output register */
+	return cc10001_adc_read_reg(adc_dev, CC10001_ADC_DDATA_OUT) &
+			       CC10001_ADC_DATA_MASK;
+}
+
+static irqreturn_t cc10001_adc_trigger_h(int irq, void *p)
+{
+	struct cc10001_adc_device *adc_dev;
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev;
+	unsigned int delay_ns;
+	unsigned int channel;
+	bool sample_invalid;
+	u16 *data;
+	int i;
+
+	indio_dev = pf->indio_dev;
+	adc_dev = iio_priv(indio_dev);
+	data = adc_dev->buf;
+
+	mutex_lock(&adc_dev->lock);
+
+	cc10001_adc_write_reg(adc_dev, CC10001_ADC_POWER_UP,
+			      CC10001_ADC_POWER_UP_SET);
+
+	/* Wait for 8 (6+2) clock cycles before activating START */
+	ndelay(adc_dev->start_delay_ns);
+
+	/* Calculate delay step for eoc and sampled data */
+	delay_ns = adc_dev->eoc_delay_ns / CC10001_MAX_POLL_COUNT;
+
+	i = 0;
+	sample_invalid = false;
+	for_each_set_bit(channel, indio_dev->active_scan_mask,
+				  indio_dev->masklength) {
+
+		cc10001_adc_start(adc_dev, channel);
+
+		data[i] = cc10001_adc_poll_done(indio_dev, channel, delay_ns);
+		if (data[i] == CC10001_INVALID_SAMPLED) {
+			dev_warn(&indio_dev->dev,
+				 "invalid sample on channel %d\n", channel);
+			sample_invalid = true;
+			goto done;
+		}
+		i++;
+	}
+
+done:
+	cc10001_adc_write_reg(adc_dev, CC10001_ADC_POWER_UP, 0);
+
+	mutex_unlock(&adc_dev->lock);
+
+	if (!sample_invalid)
+		iio_push_to_buffers_with_timestamp(indio_dev, data,
+						   iio_get_time_ns());
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
+static u16 cc10001_adc_read_raw_voltage(struct iio_dev *indio_dev,
+					struct iio_chan_spec const *chan)
+{
+	struct cc10001_adc_device *adc_dev = iio_priv(indio_dev);
+	unsigned int delay_ns;
+	u16 val;
+
+	cc10001_adc_write_reg(adc_dev, CC10001_ADC_POWER_UP,
+			      CC10001_ADC_POWER_UP_SET);
+
+	/* Wait for 8 (6+2) clock cycles before activating START */
+	ndelay(adc_dev->start_delay_ns);
+
+	/* Calculate delay step for eoc and sampled data */
+	delay_ns = adc_dev->eoc_delay_ns / CC10001_MAX_POLL_COUNT;
+
+	cc10001_adc_start(adc_dev, chan->channel);
+
+	val = cc10001_adc_poll_done(indio_dev, chan->channel, delay_ns);
+
+	cc10001_adc_write_reg(adc_dev, CC10001_ADC_POWER_UP, 0);
+
+	return val;
+}
+
+static int cc10001_adc_read_raw(struct iio_dev *indio_dev,
+				 struct iio_chan_spec const *chan,
+				 int *val, int *val2, long mask)
+{
+	struct cc10001_adc_device *adc_dev = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		if (iio_buffer_enabled(indio_dev))
+			return -EBUSY;
+		mutex_lock(&adc_dev->lock);
+		*val = cc10001_adc_read_raw_voltage(indio_dev, chan);
+		mutex_unlock(&adc_dev->lock);
+
+		if (*val == CC10001_INVALID_SAMPLED)
+			return -EIO;
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		ret = regulator_get_voltage(adc_dev->reg);
+		if (ret)
+			return ret;
+
+		*val = ret / 1000;
+		*val2 = chan->scan_type.realbits;
+		return IIO_VAL_FRACTIONAL_LOG2;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static int cc10001_update_scan_mode(struct iio_dev *indio_dev,
+				    const unsigned long *scan_mask)
+{
+	struct cc10001_adc_device *adc_dev = iio_priv(indio_dev);
+
+	kfree(adc_dev->buf);
+	adc_dev->buf = kmalloc(indio_dev->scan_bytes, GFP_KERNEL);
+	if (!adc_dev->buf)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static const struct iio_info cc10001_adc_info = {
+	.driver_module = THIS_MODULE,
+	.read_raw = &cc10001_adc_read_raw,
+	.update_scan_mode = &cc10001_update_scan_mode,
+};
+
+static int cc10001_adc_channel_init(struct iio_dev *indio_dev)
+{
+	struct cc10001_adc_device *adc_dev = iio_priv(indio_dev);
+	struct iio_chan_spec *chan_array, *timestamp;
+	unsigned int bit, idx = 0;
+
+	indio_dev->num_channels = bitmap_weight(&adc_dev->channel_map,
+						CC10001_ADC_NUM_CHANNELS);
+
+	chan_array = devm_kcalloc(&indio_dev->dev, indio_dev->num_channels + 1,
+				  sizeof(struct iio_chan_spec),
+				  GFP_KERNEL);
+	if (!chan_array)
+		return -ENOMEM;
+
+	for_each_set_bit(bit, &adc_dev->channel_map, CC10001_ADC_NUM_CHANNELS) {
+		struct iio_chan_spec *chan = &chan_array[idx];
+
+		chan->type = IIO_VOLTAGE;
+		chan->indexed = 1;
+		chan->channel = bit;
+		chan->scan_index = idx;
+		chan->scan_type.sign = 'u';
+		chan->scan_type.realbits = 10;
+		chan->scan_type.storagebits = 16;
+		chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE);
+		chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
+		idx++;
+	}
+
+	timestamp = &chan_array[idx];
+	timestamp->type = IIO_TIMESTAMP;
+	timestamp->channel = -1;
+	timestamp->scan_index = idx;
+	timestamp->scan_type.sign = 's';
+	timestamp->scan_type.realbits = 64;
+	timestamp->scan_type.storagebits = 64;
+
+	indio_dev->channels = chan_array;
+
+	return 0;
+}
+
+static int cc10001_adc_probe(struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	struct cc10001_adc_device *adc_dev;
+	unsigned long adc_clk_rate;
+	struct resource *res;
+	struct iio_dev *indio_dev;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*adc_dev));
+	if (indio_dev == NULL)
+		return -ENOMEM;
+
+	adc_dev = iio_priv(indio_dev);
+
+	adc_dev->channel_map = GENMASK(CC10001_ADC_NUM_CHANNELS - 1, 0);
+	if (!of_property_read_u32(node, "cosmic,reserved-adc-channels", &ret))
+		adc_dev->channel_map &= ~ret;
+
+	adc_dev->reg = devm_regulator_get(&pdev->dev, "vref");
+	if (IS_ERR(adc_dev->reg))
+		return PTR_ERR(adc_dev->reg);
+
+	ret = regulator_enable(adc_dev->reg);
+	if (ret)
+		return ret;
+
+	indio_dev->dev.parent = &pdev->dev;
+	indio_dev->name = dev_name(&pdev->dev);
+	indio_dev->info = &cc10001_adc_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	adc_dev->reg_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(adc_dev->reg_base)) {
+		ret = PTR_ERR(adc_dev->reg_base);
+		goto err_disable_reg;
+	}
+
+	adc_dev->adc_clk = devm_clk_get(&pdev->dev, "adc");
+	if (IS_ERR(adc_dev->adc_clk)) {
+		dev_err(&pdev->dev, "failed to get the clock\n");
+		ret = PTR_ERR(adc_dev->adc_clk);
+		goto err_disable_reg;
+	}
+
+	ret = clk_prepare_enable(adc_dev->adc_clk);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to enable the clock\n");
+		goto err_disable_reg;
+	}
+
+	adc_clk_rate = clk_get_rate(adc_dev->adc_clk);
+	if (!adc_clk_rate) {
+		ret = -EINVAL;
+		dev_err(&pdev->dev, "null clock rate!\n");
+		goto err_disable_clk;
+	}
+
+	adc_dev->eoc_delay_ns = NSEC_PER_SEC / adc_clk_rate;
+	adc_dev->start_delay_ns = adc_dev->eoc_delay_ns * CC10001_WAIT_CYCLES;
+
+	/* Setup the ADC channels available on the device */
+	ret = cc10001_adc_channel_init(indio_dev);
+	if (ret < 0)
+		goto err_disable_clk;
+
+	mutex_init(&adc_dev->lock);
+
+	ret = iio_triggered_buffer_setup(indio_dev, NULL,
+					 &cc10001_adc_trigger_h, NULL);
+	if (ret < 0)
+		goto err_disable_clk;
+
+	ret = iio_device_register(indio_dev);
+	if (ret < 0)
+		goto err_cleanup_buffer;
+
+	platform_set_drvdata(pdev, indio_dev);
+
+	return 0;
+
+err_cleanup_buffer:
+	iio_triggered_buffer_cleanup(indio_dev);
+err_disable_clk:
+	clk_disable_unprepare(adc_dev->adc_clk);
+err_disable_reg:
+	regulator_disable(adc_dev->reg);
+	return ret;
+}
+
+static int cc10001_adc_remove(struct platform_device *pdev)
+{
+	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+	struct cc10001_adc_device *adc_dev = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+	iio_triggered_buffer_cleanup(indio_dev);
+	clk_disable_unprepare(adc_dev->adc_clk);
+	regulator_disable(adc_dev->reg);
+
+	return 0;
+}
+
+static const struct of_device_id cc10001_adc_dt_ids[] = {
+	{ .compatible = "cosmic,10001-adc", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, cc10001_adc_dt_ids);
+
+static struct platform_driver cc10001_adc_driver = {
+	.driver = {
+		.name   = "cc10001-adc",
+		.owner	= THIS_MODULE,
+		.of_match_table = cc10001_adc_dt_ids,
+	},
+	.probe	= cc10001_adc_probe,
+	.remove	= cc10001_adc_remove,
+};
+module_platform_driver(cc10001_adc_driver);
+
+MODULE_AUTHOR("Phani Movva <Phani.Movva-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>");
+MODULE_DESCRIPTION("Cosmic Circuits ADC driver");
+MODULE_LICENSE("GPL v2");
-- 
2.2.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v6 2/3] DT: iio: adc: Add CC_10001 binding documentation

[Ezequiel Garcia]

From: Phani Movva <Phani.Movva-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>

Add the devicetree binding document for Cosmic Circuits 10001 ADC device.

Signed-off-by: Phani Movva <Phani.Movva-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Naidu Tellapati <Naidu.Tellapati-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
[Ezequiel: minor style cleaning]
Signed-off-by: Ezequiel Garcia <ezequiel.garcia-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
---
 .../devicetree/bindings/iio/adc/cc10001_adc.txt    | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt

diff --git a/Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt b/Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt
new file mode 100644
index 0000000..b7ba558
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt
@@ -0,0 +1,22 @@
+* Cosmic Circuits - Analog to Digital Converter (CC-10001-ADC)
+
+Required properties:
+  - compatible: Should be "cosmic,10001-adc"
+  - reg: Should contain adc registers location and length.
+  - clock-names: Should contain "adc".
+  - clocks: Should contain a clock specifier for each entry in clock-names
+  - vref-supply: The regulator supply ADC reference voltage.
+
+Optional properties:
+  - cosmic,adc-reserved-channels: Bitmask of reserved channels,
+    i.e. channels that cannot be used by the OS.
+
+Example:
+adc: adc@18101600 {
+	compatible = "cosmic,10001-adc";
+	reg = <0x18101600 0x24>;
+	cosmic,adc-reserved-channels = <0x2>;
+	clocks = <&adc_clk>;
+	clock-names = "adc";
+	vref-supply = <&reg_1v8>;
+};
-- 
2.2.1

Re: [PATCH v6 2/3] DT: iio: adc: Add CC_10001 binding documentation

[Rob Herring]

On Tue, Jan 6, 2015 at 9:29 AM, Ezequiel Garcia
<ezequiel.garcia-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org> wrote:
> From: Phani Movva <Phani.Movva-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
>
> Add the devicetree binding document for Cosmic Circuits 10001 ADC device.
>
> Signed-off-by: Phani Movva <Phani.Movva-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Naidu Tellapati <Naidu.Tellapati-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
> [Ezequiel: minor style cleaning]
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
> ---
>  .../devicetree/bindings/iio/adc/cc10001_adc.txt    | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt b/Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt
> new file mode 100644
> index 0000000..b7ba558
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt
> @@ -0,0 +1,22 @@
> +* Cosmic Circuits - Analog to Digital Converter (CC-10001-ADC)
> +
> +Required properties:
> +  - compatible: Should be "cosmic,10001-adc"
> +  - reg: Should contain adc registers location and length.
> +  - clock-names: Should contain "adc".
> +  - clocks: Should contain a clock specifier for each entry in clock-names
> +  - vref-supply: The regulator supply ADC reference voltage.
> +
> +Optional properties:
> +  - cosmic,adc-reserved-channels: Bitmask of reserved channels,
> +    i.e. channels that cannot be used by the OS.

Seems like this could be pretty common for any ADC as well as having
the number of channels, so you can drop the vendor prefix. Do we have
any similar properties already in other drivers? I'd reverse the
polarity to be enabled channels, then the number of available channels
is the top set bit plus 1. You could count channels as is, but then
people have to remember to set the high bits on non-existent channels.
Absence of the property could still mean you know how many channels
and all are enabled.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 2/3] DT: iio: adc: Add CC_10001 binding documentation

[Ezequiel Garcia]

Hi Rob,

Thanks a lot for the comments.

On 01/06/2015 01:30 PM, Rob Herring wrote:
> On Tue, Jan 6, 2015 at 9:29 AM, Ezequiel Garcia
> <ezequiel.garcia-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org> wrote:
>> From: Phani Movva <Phani.Movva-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
>>
>> Add the devicetree binding document for Cosmic Circuits 10001 ADC device.
>>
>> Signed-off-by: Phani Movva <Phani.Movva-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
>> Signed-off-by: Naidu Tellapati <Naidu.Tellapati-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
>> [Ezequiel: minor style cleaning]
>> Signed-off-by: Ezequiel Garcia <ezequiel.garcia-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
>> ---
>>  .../devicetree/bindings/iio/adc/cc10001_adc.txt    | 22 ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt b/Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt
>> new file mode 100644
>> index 0000000..b7ba558
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt
>> @@ -0,0 +1,22 @@
>> +* Cosmic Circuits - Analog to Digital Converter (CC-10001-ADC)
>> +
>> +Required properties:
>> +  - compatible: Should be "cosmic,10001-adc"
>> +  - reg: Should contain adc registers location and length.
>> +  - clock-names: Should contain "adc".
>> +  - clocks: Should contain a clock specifier for each entry in clock-names
>> +  - vref-supply: The regulator supply ADC reference voltage.
>> +
>> +Optional properties:
>> +  - cosmic,adc-reserved-channels: Bitmask of reserved channels,
>> +    i.e. channels that cannot be used by the OS.
> 
> Seems like this could be pretty common for any ADC as well as having
> the number of channels, so you can drop the vendor prefix. Do we have
> any similar properties already in other drivers?

Well, it seems the AT91 ADC has a similar property called
adc-channels-used. Not sure if it means the same as here. We are not
using it to model a hardware availability but the channels being
reserved for another core or OS. See [1].

> I'd reverse the
> polarity to be enabled channels, then the number of available channels
> is the top set bit plus 1. You could count channels as is, but then
> people have to remember to set the high bits on non-existent channels.

Hm, isn't this too cumbersome? If we really need to pass the number of
channels, shouldn't we have a separate property for that? Note that no
driver currently needs it.

> Absence of the property could still mean you know how many channels
> and all are enabled.
> 

This property was originally proposed to be "called
cosmic,adc-available-channels". Andrew suggested to reverse the
polarity, and be explicit about absence of property meaning no reserved
channels (also in [1]).

I'm fine either way.

[1] http://www.spinics.net/lists/linux-iio/msg15578.html
-- 
Ezequiel
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 2/3] DT: iio: adc: Add CC_10001 binding documentation

[Rob Herring]

On Tue, Jan 6, 2015 at 10:59 AM, Ezequiel Garcia
<ezequiel.garcia-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org> wrote:
> Hi Rob,
>
> Thanks a lot for the comments.
>
> On 01/06/2015 01:30 PM, Rob Herring wrote:
>> On Tue, Jan 6, 2015 at 9:29 AM, Ezequiel Garcia
>> <ezequiel.garcia-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org> wrote:
>>> From: Phani Movva <Phani.Movva-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
>>>
>>> Add the devicetree binding document for Cosmic Circuits 10001 ADC device.
>>>
>>> Signed-off-by: Phani Movva <Phani.Movva-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
>>> Signed-off-by: Naidu Tellapati <Naidu.Tellapati-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
>>> [Ezequiel: minor style cleaning]
>>> Signed-off-by: Ezequiel Garcia <ezequiel.garcia-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
>>> ---
>>>  .../devicetree/bindings/iio/adc/cc10001_adc.txt    | 22 ++++++++++++++++++++++
>>>  1 file changed, 22 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt b/Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt
>>> new file mode 100644
>>> index 0000000..b7ba558
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt
>>> @@ -0,0 +1,22 @@
>>> +* Cosmic Circuits - Analog to Digital Converter (CC-10001-ADC)
>>> +
>>> +Required properties:
>>> +  - compatible: Should be "cosmic,10001-adc"
>>> +  - reg: Should contain adc registers location and length.
>>> +  - clock-names: Should contain "adc".
>>> +  - clocks: Should contain a clock specifier for each entry in clock-names
>>> +  - vref-supply: The regulator supply ADC reference voltage.
>>> +
>>> +Optional properties:
>>> +  - cosmic,adc-reserved-channels: Bitmask of reserved channels,
>>> +    i.e. channels that cannot be used by the OS.
>>
>> Seems like this could be pretty common for any ADC as well as having
>> the number of channels, so you can drop the vendor prefix. Do we have
>> any similar properties already in other drivers?
>
> Well, it seems the AT91 ADC has a similar property called
> adc-channels-used. Not sure if it means the same as here. We are not
> using it to model a hardware availability but the channels being
> reserved for another core or OS. See [1].
>
>> I'd reverse the
>> polarity to be enabled channels, then the number of available channels
>> is the top set bit plus 1. You could count channels as is, but then
>> people have to remember to set the high bits on non-existent channels.
>
> Hm, isn't this too cumbersome? If we really need to pass the number of
> channels, shouldn't we have a separate property for that? Note that no
> driver currently needs it.

Okay.

>> Absence of the property could still mean you know how many channels
>> and all are enabled.
>>
>
> This property was originally proposed to be "called
> cosmic,adc-available-channels". Andrew suggested to reverse the
> polarity, and be explicit about absence of property meaning no reserved
> channels (also in [1]).

Okay, that's fine. I'd still suggest dropping cosmic. With that:

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Rob

[PATCH v6 3/3] DT: Add a vendor prefix for Cosmic Circuits

[Ezequiel Garcia]

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index b1df0ad..014770f 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -36,6 +36,7 @@ chunghwa	Chunghwa Picture Tubes Ltd.
 cirrus	Cirrus Logic, Inc.
 cnm	Chips&Media, Inc.
 cortina	Cortina Systems, Inc.
+cosmic	Cosmic Circuits
 crystalfontz	Crystalfontz America, Inc.
 dallas	Maxim Integrated Products (formerly Dallas Semiconductor)
 davicom	DAVICOM Semiconductor, Inc.
-- 
2.2.1