linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv3 0/3] Add support for the Nuvoton NAU7802 ADC to the cfa10049
@ 2013-06-24 17:24 Alexandre Belloni
  2013-06-24 17:24 ` [PATCHv3 1/3] iio: Add Nuvoton NAU7802 ADC driver Alexandre Belloni
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Alexandre Belloni @ 2013-06-24 17:24 UTC (permalink / raw)
  To: Shawn Guo, Jonathan Cameron
  Cc: jimwall, brian, Maxime Ripard, linux-iio, linux-kernel,
	linux-doc, linux-arm-kernel, devicetree-discuss, Rob Landley,
	Rob Herring, Grant Likely, Alexandre Belloni

Hi,

This is v3 of the patchset that adds support for the 3 Nuvoton NAU7802 ADCs
present on the CFA-10049 board from Crystalfontz. The first patch adds the
driver, the next ones are adding them in the DT after switching the second i2c
bus to bitbanging following a lot of timeout issues we have on that bus.

Changes in v3:
 - removed the min_conversion knob in sysfs, it is now hardcoded to 6
 - added error checking in probe
 - corrected error checking
 - corrected possibly uninitialized vldo
 - used ARRAY_SIZE(nau7802_chan_array) instead of 2
 - added that vldo refers to the internal voltage regulator

Changes in v2:
 - channels are now statically allocated
 - no wrappers around i2c functions
 - sampling_freq is a shared attribute
 - setting gain is done through scale
 - cosmetics changes (s/idev/indio_dev/, blank lines before returns)
 - use of sign_extend32 to extend the sign
 - reading scale now returns IIO_VAL_FRACTIONAL_LOG2
 - no dt lookup for interrupts, this is done in of_i2c.c but that was mask due
 to a bug.

what I didn't address is setting the external reference voltage through a fixed
regulator and I kept the vldo attribute to set the internal voltage reference in
the DT as Jonathan seemed to be ok with that. If anybody is using a nau7802 with
an external reference, it will still be time to add the fixed regulator. It
shouldn't be difficult but I don't have any hardware to test.

Regards,

Alexandre Belloni (1):
  iio: Add Nuvoton NAU7802 ADC driver

Maxime Ripard (2):
  ARM: mxs: cfa10049: Switch bus i2c1 to bitbanging
  ARM: mxs: cfa10049: Add NAU7802 ADCs to the device tree

 .../bindings/iio/adc/nuvoton-nau7802.txt           |  18 +
 arch/arm/boot/dts/imx28-cfa10049.dts               | 126 +++--
 drivers/iio/adc/Kconfig                            |   9 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/nau7802.c                          | 576 +++++++++++++++++++++
 5 files changed, 687 insertions(+), 43 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton-nau7802.txt
 create mode 100644 drivers/iio/adc/nau7802.c

-- 
1.8.1.2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCHv3 1/3] iio: Add Nuvoton NAU7802 ADC driver
  2013-06-24 17:24 [PATCHv3 0/3] Add support for the Nuvoton NAU7802 ADC to the cfa10049 Alexandre Belloni
@ 2013-06-24 17:24 ` Alexandre Belloni
  2013-07-04  9:08   ` Lars-Peter Clausen
  2013-06-24 17:24 ` [PATCHv3 2/3] ARM: mxs: cfa10049: Switch bus i2c1 to bitbanging Alexandre Belloni
  2013-06-24 17:24 ` [PATCHv3 3/3] ARM: mxs: cfa10049: Add NAU7802 ADCs to the device tree Alexandre Belloni
  2 siblings, 1 reply; 17+ messages in thread
From: Alexandre Belloni @ 2013-06-24 17:24 UTC (permalink / raw)
  To: Shawn Guo, Jonathan Cameron
  Cc: jimwall, brian, Maxime Ripard, linux-iio, linux-kernel,
	linux-doc, linux-arm-kernel, devicetree-discuss, Rob Landley,
	Rob Herring, Grant Likely, Alexandre Belloni

The Nuvoton NAU7802 ADC is a 24-bit 2-channels I2C ADC, with adjustable
gain and sampling rates.

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 .../bindings/iio/adc/nuvoton-nau7802.txt           |  18 +
 drivers/iio/adc/Kconfig                            |   9 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/nau7802.c                          | 576 +++++++++++++++++++++
 4 files changed, 604 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton-nau7802.txt
 create mode 100644 drivers/iio/adc/nau7802.c

diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton-nau7802.txt b/Documentation/devicetree/bindings/iio/adc/nuvoton-nau7802.txt
new file mode 100644
index 0000000..e9582e6
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/nuvoton-nau7802.txt
@@ -0,0 +1,18 @@
+* Nuvoton NAU7802 Analog to Digital Converter (ADC)
+
+Required properties:
+  - compatible: Should be "nuvoton,nau7802"
+  - reg: Should contain the ADC I2C address
+
+Optional properties:
+  - nuvoton,vldo: Internal reference voltage in millivolts to be
+    configured valid values are between 2400 mV and 4500 mV.
+  - interrupts: IRQ line for the ADC. If not used the driver will use
+    polling.
+
+Example:
+adc2: nau7802@2a {
+	compatible = "nuvoton,nau7802";
+	reg = <0x2a>;
+	nuvoton,vldo = <3000>;
+};
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index ab0767e6..d7f9ed8 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -133,6 +133,15 @@ config MAX1363
 	  max11646, max11647) Provides direct access via sysfs and buffered
 	  data via the iio dev interface.
 
+config NAU7802
+	tristate "Nuvoton NAU7802 ADC driver"
+	depends on I2C
+	help
+	  Say yes here to build support for Nuvoton NAU7802 ADC.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called nau7802.
+
 config TI_ADC081C
 	tristate "Texas Instruments ADC081C021/027"
 	depends on I2C
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 0a825be..d426081 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_AT91_ADC) += at91_adc.o
 obj-$(CONFIG_EXYNOS_ADC) += exynos_adc.o
 obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
 obj-$(CONFIG_MAX1363) += max1363.o
+obj-$(CONFIG_NAU7802) += nau7802.o
 obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
 obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
 obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o
diff --git a/drivers/iio/adc/nau7802.c b/drivers/iio/adc/nau7802.c
new file mode 100644
index 0000000..c5f99a2
--- /dev/null
+++ b/drivers/iio/adc/nau7802.c
@@ -0,0 +1,576 @@
+/*
+ * Driver for the Nuvoton NAU7802 ADC
+ *
+ * Copyright 2013 Free Electrons
+ *
+ * Licensed under the GPLv2 or later.
+ */
+
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/wait.h>
+#include <linux/log2.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+#define NAU7802_REG_PUCTRL	0x00
+#define NAU7802_PUCTRL_RR(x)		(x << 0)
+#define NAU7802_PUCTRL_RR_BIT		NAU7802_PUCTRL_RR(1)
+#define NAU7802_PUCTRL_PUD(x)		(x << 1)
+#define NAU7802_PUCTRL_PUD_BIT		NAU7802_PUCTRL_PUD(1)
+#define NAU7802_PUCTRL_PUA(x)		(x << 2)
+#define NAU7802_PUCTRL_PUA_BIT		NAU7802_PUCTRL_PUA(1)
+#define NAU7802_PUCTRL_PUR(x)		(x << 3)
+#define NAU7802_PUCTRL_PUR_BIT		NAU7802_PUCTRL_PUR(1)
+#define NAU7802_PUCTRL_CS(x)		(x << 4)
+#define NAU7802_PUCTRL_CS_BIT		NAU7802_PUCTRL_CS(1)
+#define NAU7802_PUCTRL_CR(x)		(x << 5)
+#define NAU7802_PUCTRL_CR_BIT		NAU7802_PUCTRL_CR(1)
+#define NAU7802_PUCTRL_AVDDS(x)		(x << 7)
+#define NAU7802_PUCTRL_AVDDS_BIT	NAU7802_PUCTRL_AVDDS(1)
+#define NAU7802_REG_CTRL1	0x01
+#define NAU7802_CTRL1_VLDO(x)		(x << 3)
+#define NAU7802_CTRL1_GAINS(x)		(x)
+#define NAU7802_CTRL1_GAINS_BITS	0x07
+#define NAU7802_REG_CTRL2	0x02
+#define NAU7802_CTRL2_CHS(x)		(x << 7)
+#define NAU7802_CTRL2_CRS(x)		(x << 4)
+#define NAU7802_SAMP_FREQ_320	0x07
+#define NAU7802_CTRL2_CHS_BIT		NAU7802_CTRL2_CHS(1)
+#define NAU7802_REG_ADC_B2	0x12
+#define NAU7802_REG_ADC_B1	0x13
+#define NAU7802_REG_ADC_B0	0x14
+#define NAU7802_REG_ADC_CTRL	0x15
+
+#define NAU7802_MIN_CONVERSIONS 6
+
+struct nau7802_state {
+	struct i2c_client	*client;
+	s32			last_value;
+	struct mutex		lock;
+	struct mutex		data_lock;
+	u32			vref_mv;
+	u32			conversion_count;
+	u32			min_conversions;
+	u8			sample_rate;
+	u32			scale_avail[8];
+	struct completion	value_ok;
+};
+
+#define NAU7802_CHANNEL(chan) {					\
+	.type = IIO_VOLTAGE,					\
+	.indexed = 1,						\
+	.channel = (chan),					\
+	.scan_index = (chan),					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
+				BIT(IIO_CHAN_INFO_SAMP_FREQ)	\
+}
+
+static const struct iio_chan_spec nau7802_chan_array[] = {
+	NAU7802_CHANNEL(0),
+	NAU7802_CHANNEL(1),
+};
+
+static const u16 nau7802_sample_freq_avail[] = {10, 20, 40, 80,
+						10, 10, 10, 320};
+
+static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("10 40 80 320");
+
+static struct attribute *nau7802_attributes[] = {
+	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group nau7802_attribute_group = {
+	.attrs = nau7802_attributes,
+};
+
+static int nau7802_set_gain(struct nau7802_state *st, int gain)
+{
+	int ret;
+
+	mutex_lock(&st->lock);
+	st->conversion_count = 0;
+
+	ret = i2c_smbus_read_byte_data(st->client, NAU7802_REG_CTRL1);
+	if (ret < 0)
+		goto nau7802_sysfs_set_gain_out;
+	ret = i2c_smbus_write_byte_data(st->client, NAU7802_REG_CTRL1,
+					(ret & (~NAU7802_CTRL1_GAINS_BITS)) |
+					gain);
+
+nau7802_sysfs_set_gain_out:
+	mutex_unlock(&st->lock);
+
+	return ret;
+}
+
+static int nau7802_read_conversion(struct nau7802_state *st)
+{
+	int data;
+
+	mutex_lock(&st->data_lock);
+	data = i2c_smbus_read_byte_data(st->client, NAU7802_REG_ADC_B2);
+	if (data < 0)
+		goto nau7802_read_conversion_out;
+	st->last_value = data << 16;
+
+	data = i2c_smbus_read_byte_data(st->client, NAU7802_REG_ADC_B1);
+	if (data < 0)
+		goto nau7802_read_conversion_out;
+	st->last_value |= data << 8;
+
+	data = i2c_smbus_read_byte_data(st->client, NAU7802_REG_ADC_B0);
+	if (data < 0)
+		goto nau7802_read_conversion_out;
+	st->last_value |= data;
+
+	st->last_value = sign_extend32(st->last_value, 23);
+
+nau7802_read_conversion_out:
+	mutex_unlock(&st->data_lock);
+
+	return data;
+}
+
+/*
+ * Conversions are synchronised on the rising edge of NAU7802_PUCTRL_CS_BIT
+ */
+static int nau7802_sync(struct nau7802_state *st)
+{
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(st->client, NAU7802_REG_PUCTRL);
+	if (ret < 0)
+		return ret;
+	ret = i2c_smbus_write_byte_data(st->client, NAU7802_REG_PUCTRL,
+				ret | NAU7802_PUCTRL_CS_BIT);
+
+	return ret;
+}
+
+static irqreturn_t nau7802_eoc_trigger(int irq, void *private)
+{
+	struct iio_dev *indio_dev = private;
+	struct nau7802_state *st = iio_priv(indio_dev);
+	int status;
+
+	status = i2c_smbus_read_byte_data(st->client, NAU7802_REG_PUCTRL);
+	if (status < 0)
+		return IRQ_HANDLED;
+
+	if (!(status & NAU7802_PUCTRL_CR_BIT))
+		return IRQ_NONE;
+
+	if (nau7802_read_conversion(st) < 0)
+		return IRQ_HANDLED;
+
+	/* Because there is actually only one ADC for both channels, we have to
+	 * wait for enough conversions to happen before getting a significant
+	 * value when changing channels and the values are far apart.
+	 */
+	if (st->conversion_count < NAU7802_MIN_CONVERSIONS)
+		st->conversion_count++;
+	if (st->conversion_count >= NAU7802_MIN_CONVERSIONS)
+		complete_all(&st->value_ok);
+
+	return IRQ_HANDLED;
+}
+
+static int nau7802_read_irq(struct iio_dev *indio_dev,
+			struct iio_chan_spec const *chan,
+			int *val)
+{
+	struct nau7802_state *st = iio_priv(indio_dev);
+	int ret;
+
+	INIT_COMPLETION(st->value_ok);
+	enable_irq(st->client->irq);
+
+	nau7802_sync(st);
+
+	/* read registers to ensure we flush everything */
+	ret = nau7802_read_conversion(st);
+	if (ret < 0)
+		goto read_chan_info_failure;
+
+	/* Wait for a conversion to finish */
+	ret = wait_for_completion_interruptible_timeout(&st->value_ok,
+			msecs_to_jiffies(1000));
+	if (ret == 0)
+		ret = -ETIMEDOUT;
+
+	if (ret < 0)
+		goto read_chan_info_failure;
+
+	disable_irq(st->client->irq);
+
+	*val = st->last_value;
+
+	return IIO_VAL_INT;
+
+read_chan_info_failure:
+	disable_irq(st->client->irq);
+
+	return ret;
+}
+
+static int nau7802_read_poll(struct iio_dev *indio_dev,
+			struct iio_chan_spec const *chan,
+			int *val)
+{
+	struct nau7802_state *st = iio_priv(indio_dev);
+	int ret;
+
+	nau7802_sync(st);
+
+	/* read registers to ensure we flush everything */
+	ret = nau7802_read_conversion(st);
+	if (ret < 0)
+		return ret;
+
+	/* Because there is actually only one ADC for both channels, we have to
+	 * wait for enough conversions to happen before getting a significant
+	 * value when changing channels and the values are far appart.
+	 */
+	do {
+		ret = i2c_smbus_read_byte_data(st->client, NAU7802_REG_PUCTRL);
+		if (ret < 0)
+			return ret;
+
+		while (!(ret & NAU7802_PUCTRL_CR_BIT)) {
+			if (st->sample_rate != NAU7802_SAMP_FREQ_320)
+				msleep(20);
+			else
+				mdelay(4);
+			ret = i2c_smbus_read_byte_data(st->client,
+							NAU7802_REG_PUCTRL);
+			if (ret < 0)
+				return ret;
+		}
+
+		ret = nau7802_read_conversion(st);
+		if (ret < 0)
+			return ret;
+		if (st->conversion_count < NAU7802_MIN_CONVERSIONS)
+			st->conversion_count++;
+	} while (st->conversion_count < NAU7802_MIN_CONVERSIONS);
+
+	*val = st->last_value;
+
+	return IIO_VAL_INT;
+}
+
+static int nau7802_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	struct nau7802_state *st = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&st->lock);
+		/*
+		 * Select the channel to use
+		 *   - Channel 1 is value 0 in the CHS register
+		 *   - Channel 2 is value 1 in the CHS register
+		 */
+		ret = i2c_smbus_read_byte_data(st->client, NAU7802_REG_CTRL2);
+		if (ret < 0)
+			return ret;
+
+		if (((ret & NAU7802_CTRL2_CHS_BIT) && !chan->channel) ||
+				(!(ret & NAU7802_CTRL2_CHS_BIT) &&
+				 chan->channel)) {
+			st->conversion_count = 0;
+			ret = i2c_smbus_write_byte_data(st->client,
+					NAU7802_REG_CTRL2,
+					NAU7802_CTRL2_CHS(chan->channel) |
+					NAU7802_CTRL2_CRS(st->sample_rate));
+
+			if (ret < 0)
+				return ret;
+		}
+
+		if (st->client->irq)
+			ret = nau7802_read_irq(indio_dev, chan, val);
+		else
+			ret = nau7802_read_poll(indio_dev, chan, val);
+
+		mutex_unlock(&st->lock);
+		return ret;
+
+	case IIO_CHAN_INFO_SCALE:
+		ret = i2c_smbus_read_byte_data(st->client, NAU7802_REG_CTRL1);
+		if (ret < 0)
+			return ret;
+
+		/* we have 24 bits of signed data, that means 23 bits of data
+		 * plus the sign bit */
+		*val = st->vref_mv;
+		*val2 = 23 + (ret & NAU7802_CTRL1_GAINS_BITS);
+
+		return IIO_VAL_FRACTIONAL_LOG2;
+
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*val =  nau7802_sample_freq_avail[st->sample_rate];
+		*val2 = 0;
+		return IIO_VAL_INT;
+
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static int nau7802_write_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int val, int val2, long mask)
+{
+	struct nau7802_state *st = iio_priv(indio_dev);
+	int i, ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		for (i = 0; i < ARRAY_SIZE(st->scale_avail); i++)
+			if (val2 == st->scale_avail[i])
+				return nau7802_set_gain(st, i);
+
+		break;
+
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		for (i = 0; i < ARRAY_SIZE(nau7802_sample_freq_avail); i++)
+			if (val == nau7802_sample_freq_avail[i]) {
+				mutex_lock(&st->lock);
+				st->sample_rate = i;
+				st->conversion_count = 0;
+				ret = i2c_smbus_write_byte_data(st->client,
+					NAU7802_REG_CTRL2,
+					NAU7802_CTRL2_CRS(st->sample_rate));
+				mutex_unlock(&st->lock);
+				return ret;
+			}
+
+		break;
+
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static int nau7802_write_raw_get_fmt(struct iio_dev *indio_dev,
+				     struct iio_chan_spec const *chan,
+				     long mask)
+{
+	return IIO_VAL_INT_PLUS_NANO;
+}
+
+static const struct iio_info nau7802_info = {
+	.driver_module = THIS_MODULE,
+	.read_raw = &nau7802_read_raw,
+	.write_raw = &nau7802_write_raw,
+	.write_raw_get_fmt = nau7802_write_raw_get_fmt,
+	.attrs = &nau7802_attribute_group,
+};
+
+static int nau7802_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct iio_dev *indio_dev;
+	struct nau7802_state *st;
+	struct device_node *np = client->dev.of_node;
+	int i, ret;
+	u8 data;
+	u32 tmp = 0;
+
+	if (!client->dev.of_node) {
+		dev_err(&client->dev, "No device tree node available.\n");
+		return -EINVAL;
+	}
+
+	indio_dev = iio_device_alloc(sizeof(*st));
+	if (indio_dev == NULL)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+
+	i2c_set_clientdata(client, indio_dev);
+
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->name = dev_name(&client->dev);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->info = &nau7802_info;
+
+	st->client = client;
+
+	/* Reset the device */
+	ret = i2c_smbus_write_byte_data(st->client, NAU7802_REG_PUCTRL,
+				  NAU7802_PUCTRL_RR_BIT);
+	if (ret < 0)
+		goto error_free_indio;
+
+	/* Enter normal operation mode */
+	ret = i2c_smbus_write_byte_data(st->client, NAU7802_REG_PUCTRL,
+				  NAU7802_PUCTRL_PUD_BIT);
+	if (ret < 0)
+		goto error_free_indio;
+
+	/*
+	 * After about 200 usecs, the device should be ready and then
+	 * the Power Up bit will be set to 1. If not, wait for it.
+	 */
+	udelay(210);
+	ret = i2c_smbus_read_byte_data(st->client, NAU7802_REG_PUCTRL);
+	if (ret < 0)
+		goto error_free_indio;
+	if (!(ret & NAU7802_PUCTRL_PUR_BIT))
+		goto error_free_indio;
+
+	of_property_read_u32(np, "nuvoton,vldo", &tmp);
+	st->vref_mv = tmp;
+
+	data = NAU7802_PUCTRL_PUD_BIT | NAU7802_PUCTRL_PUA_BIT |
+		NAU7802_PUCTRL_CS_BIT;
+	if (tmp >= 2400)
+		data |= NAU7802_PUCTRL_AVDDS_BIT;
+
+	ret = i2c_smbus_write_byte_data(st->client, NAU7802_REG_PUCTRL, data);
+	if (ret < 0)
+		goto error_free_indio;
+	ret = i2c_smbus_write_byte_data(st->client, NAU7802_REG_ADC_CTRL, 0x30);
+	if (ret < 0)
+		goto error_free_indio;
+
+	if (tmp >= 2400) {
+		data = NAU7802_CTRL1_VLDO((4500 - tmp) / 300);
+		ret = i2c_smbus_write_byte_data(st->client, NAU7802_REG_CTRL1,
+						data);
+		if (ret < 0)
+			goto error_free_indio;
+	}
+
+	/* Populate available ADC input ranges */
+	for (i = 0; i < ARRAY_SIZE(st->scale_avail); i++)
+		st->scale_avail[i] = (((u64)st->vref_mv) * 1000000000ULL)
+					   >> (23 + i);
+
+	init_completion(&st->value_ok);
+
+	/*
+	 * The ADC fires continuously and we can't do anything about
+	 * it. So we need to have the IRQ disabled by default, and we
+	 * will enable them back when we will need them..
+	 */
+	if (client->irq) {
+		ret = request_threaded_irq(client->irq,
+				NULL,
+				nau7802_eoc_trigger,
+				IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+				client->dev.driver->name,
+				indio_dev);
+		if (ret) {
+			/*
+			 * What may happen here is that our IRQ controller is
+			 * not able to get level interrupt but this is required
+			 * by this ADC as when going over 40 sample per second,
+			 * the interrupt line may stay high between conversions.
+			 * So, we continue no matter what but we switch to
+			 * polling mode.
+			 */
+			dev_info(&client->dev,
+				"Failed to allocate IRQ, using polling mode\n");
+			client->irq = 0;
+		} else
+			disable_irq(client->irq);
+	}
+
+	if (!client->irq) {
+		/*
+		 * We are polling, use the fastest sample rate by
+		 * default
+		 */
+		st->sample_rate = NAU7802_SAMP_FREQ_320;
+		ret = i2c_smbus_write_byte_data(st->client, NAU7802_REG_CTRL2,
+					  NAU7802_CTRL2_CRS(st->sample_rate));
+		if (ret)
+			goto error_free_irq;
+	}
+
+	/* Setup the ADC channels available on the board */
+	indio_dev->num_channels = ARRAY_SIZE(nau7802_chan_array);
+	indio_dev->channels = nau7802_chan_array;
+
+	mutex_init(&st->lock);
+	mutex_init(&st->data_lock);
+
+	ret = iio_device_register(indio_dev);
+	if (ret < 0) {
+		dev_err(&client->dev, "Couldn't register the device.\n");
+		goto error_device_register;
+	}
+
+	return 0;
+
+error_device_register:
+	mutex_destroy(&st->lock);
+	mutex_destroy(&st->data_lock);
+error_free_irq:
+	if (client->irq)
+		free_irq(client->irq, indio_dev);
+error_free_indio:
+	iio_device_free(indio_dev);
+
+	return ret;
+}
+
+static int nau7802_remove(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+	struct nau7802_state *st = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+	mutex_destroy(&st->lock);
+	mutex_destroy(&st->data_lock);
+	if (client->irq)
+		free_irq(client->irq, indio_dev);
+	iio_device_free(indio_dev);
+
+	return 0;
+}
+
+static const struct i2c_device_id nau7802_i2c_id[] = {
+	{ "nau7802", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, nau7802_i2c_id);
+
+static const struct of_device_id nau7802_dt_ids[] = {
+	{ .compatible = "nuvoton,nau7802" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, nau7802_dt_ids);
+
+static struct i2c_driver nau7802_driver = {
+	.probe = nau7802_probe,
+	.remove = nau7802_remove,
+	.id_table = nau7802_i2c_id,
+	.driver = {
+		   .name = "nau7802",
+		   .of_match_table = of_match_ptr(nau7802_dt_ids),
+	},
+};
+
+module_i2c_driver(nau7802_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Nuvoton NAU7802 ADC Driver");
+MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
+MODULE_AUTHOR("Alexandre Belloni <alexandre.belloni@free-electrons.com>");
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCHv3 2/3] ARM: mxs: cfa10049: Switch bus i2c1 to bitbanging
  2013-06-24 17:24 [PATCHv3 0/3] Add support for the Nuvoton NAU7802 ADC to the cfa10049 Alexandre Belloni
  2013-06-24 17:24 ` [PATCHv3 1/3] iio: Add Nuvoton NAU7802 ADC driver Alexandre Belloni
@ 2013-06-24 17:24 ` Alexandre Belloni
  2013-07-02  2:45   ` Fabio Estevam
  2013-07-06 10:26   ` Jonathan Cameron
  2013-06-24 17:24 ` [PATCHv3 3/3] ARM: mxs: cfa10049: Add NAU7802 ADCs to the device tree Alexandre Belloni
  2 siblings, 2 replies; 17+ messages in thread
From: Alexandre Belloni @ 2013-06-24 17:24 UTC (permalink / raw)
  To: Shawn Guo, Jonathan Cameron
  Cc: jimwall, brian, Maxime Ripard, linux-iio, linux-kernel,
	linux-doc, linux-arm-kernel, devicetree-discuss, Rob Landley,
	Rob Herring, Grant Likely, Alexandre Belloni

From: Maxime Ripard <maxime.ripard@free-electrons.com>

The ADCs connected to this bus have been experiencing some timeout
issues when using the iMX28 i2c controller. Switching back to bitbanging
solves this.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
 arch/arm/boot/dts/imx28-cfa10049.dts | 108 +++++++++++++++++++++--------------
 1 file changed, 65 insertions(+), 43 deletions(-)

diff --git a/arch/arm/boot/dts/imx28-cfa10049.dts b/arch/arm/boot/dts/imx28-cfa10049.dts
index 05ae549..d3758c2 100644
--- a/arch/arm/boot/dts/imx28-cfa10049.dts
+++ b/arch/arm/boot/dts/imx28-cfa10049.dts
@@ -139,6 +139,17 @@
 					fsl,pull-up = <0>; /* 0 will enable the keeper */
 				};
 
+				i2c1_pins_cfa10049: i2c1@0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						0x3103 /* MX28_PAD_PWM0__GPIO */
+						0x3113 /* MX28_PAD_PWM1__I2C1_SDA */
+					>;
+					fsl,drive-strength = <1>;
+					fsl,voltage = <1>;
+					fsl,pull-up = <1>;
+				};
+
 				fiq_pins_cfa10049: fiq@0 {
 					reg = <0>;
 					fsl,pinmux-ids = <
@@ -199,49 +210,6 @@
 				status = "okay";
 			};
 
-			i2c1: i2c@8005a000 {
-				pinctrl-names = "default";
-				pinctrl-0 = <&i2c1_pins_a>;
-				status = "okay";
-			};
-
-			i2cmux {
-				compatible = "i2c-mux-gpio";
-				#address-cells = <1>;
-				#size-cells = <0>;
-				mux-gpios = <&gpio1 22 0 &gpio1 23 0>;
-				i2c-parent = <&i2c1>;
-
-				i2c@0 {
-					reg = <0>;
-				};
-
-				i2c@1 {
-					reg = <1>;
-				};
-
-				i2c@2 {
-					reg = <2>;
-				};
-
-				i2c@3 {
-					reg = <3>;
-					#address-cells = <1>;
-					#size-cells = <0>;
-
-					pca9555: pca9555@20 {
-						compatible = "nxp,pca9555";
-						interrupt-parent = <&gpio2>;
-						interrupts = <19 0x2>;
-						gpio-controller;
-						#gpio-cells = <2>;
-						interrupt-controller;
-						#interrupt-cells = <2>;
-						reg = <0x20>;
-					};
-				};
-			};
-
 			usbphy1: usbphy@8007e000 {
 				status = "okay";
 			};
@@ -366,6 +334,60 @@
 		rotary-encoder,relative-axis;
 	};
 
+	i2c1gpio: i2c@0 {
+		compatible = "i2c-gpio";
+		pinctrl-0 = <&i2c1_pins_cfa10049>;
+		pinctrl-names = "default";
+		gpios = <
+			&gpio3 17 0 /* sda */
+			&gpio3 16 0 /* scl */
+			 >;
+		i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+	};
+
+	i2cmux {
+		compatible = "i2c-mux-gpio";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		mux-gpios = <&gpio1 22 0 &gpio1 23 0>;
+		i2c-parent = <&i2c1gpio>;
+
+		i2c@0 {
+			reg = <0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		i2c@1 {
+			reg = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		i2c@2 {
+			reg = <2>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		i2c@3 {
+			reg = <3>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			pca9555: pca9555@20 {
+				compatible = "nxp,pca9555";
+				interrupt-parent = <&gpio2>;
+				interrupts = <19 0x2>;
+				gpio-controller;
+				#gpio-cells = <2>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				reg = <0x20>;
+			};
+		};
+	};
+
 	backlight {
 		compatible = "pwm-backlight";
 		pwms = <&pwm 3 5000000>;
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCHv3 3/3] ARM: mxs: cfa10049: Add NAU7802 ADCs to the device tree
  2013-06-24 17:24 [PATCHv3 0/3] Add support for the Nuvoton NAU7802 ADC to the cfa10049 Alexandre Belloni
  2013-06-24 17:24 ` [PATCHv3 1/3] iio: Add Nuvoton NAU7802 ADC driver Alexandre Belloni
  2013-06-24 17:24 ` [PATCHv3 2/3] ARM: mxs: cfa10049: Switch bus i2c1 to bitbanging Alexandre Belloni
@ 2013-06-24 17:24 ` Alexandre Belloni
  2 siblings, 0 replies; 17+ messages in thread
From: Alexandre Belloni @ 2013-06-24 17:24 UTC (permalink / raw)
  To: Shawn Guo, Jonathan Cameron
  Cc: jimwall, brian, Maxime Ripard, linux-iio, linux-kernel,
	linux-doc, linux-arm-kernel, devicetree-discuss, Rob Landley,
	Rob Herring, Grant Likely, Alexandre Belloni

From: Maxime Ripard <maxime.ripard@free-electrons.com>

Since these ADCs share the same non-configurable address on the I2C bus,
they have to be put behind a muxer.

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/imx28-cfa10049.dts | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm/boot/dts/imx28-cfa10049.dts b/arch/arm/boot/dts/imx28-cfa10049.dts
index d3758c2..efc4213 100644
--- a/arch/arm/boot/dts/imx28-cfa10049.dts
+++ b/arch/arm/boot/dts/imx28-cfa10049.dts
@@ -356,18 +356,36 @@
 			reg = <0>;
 			#address-cells = <1>;
 			#size-cells = <0>;
+
+			adc0: nau7802@2a {
+				compatible = "nuvoton,nau7802";
+				reg = <0x2a>;
+				nuvoton,vldo = <3000>;
+			};
 		};
 
 		i2c@1 {
 			reg = <1>;
 			#address-cells = <1>;
 			#size-cells = <0>;
+
+			adc1: nau7802@2a {
+				compatible = "nuvoton,nau7802";
+				reg = <0x2a>;
+				nuvoton,vldo = <3000>;
+			};
 		};
 
 		i2c@2 {
 			reg = <2>;
 			#address-cells = <1>;
 			#size-cells = <0>;
+
+			adc2: nau7802@2a {
+				compatible = "nuvoton,nau7802";
+				reg = <0x2a>;
+				nuvoton,vldo = <3000>;
+			};
 		};
 
 		i2c@3 {
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCHv3 2/3] ARM: mxs: cfa10049: Switch bus i2c1 to bitbanging
  2013-06-24 17:24 ` [PATCHv3 2/3] ARM: mxs: cfa10049: Switch bus i2c1 to bitbanging Alexandre Belloni
@ 2013-07-02  2:45   ` Fabio Estevam
  2013-07-02 11:35     ` Alexandre Belloni
  2013-07-06 10:26   ` Jonathan Cameron
  1 sibling, 1 reply; 17+ messages in thread
From: Fabio Estevam @ 2013-07-02  2:45 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Shawn Guo, Jonathan Cameron, brian, linux-doc, linux-iio,
	devicetree-discuss, jimwall, Rob Herring, linux-kernel,
	Grant Likely, Rob Landley, Maxime Ripard, linux-arm-kernel

Hi Alexandre,

On Mon, Jun 24, 2013 at 2:24 PM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> From: Maxime Ripard <maxime.ripard@free-electrons.com>
>
> The ADCs connected to this bus have been experiencing some timeout
> issues when using the iMX28 i2c controller. Switching back to bitbanging
> solves this.

Are you able to use the mxs i2c controller instead of bitbang if you
use this patch?
http://www.spinics.net/lists/stable/msg13202.html

Regards,

Fabio Estevam

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCHv3 2/3] ARM: mxs: cfa10049: Switch bus i2c1 to bitbanging
  2013-07-02  2:45   ` Fabio Estevam
@ 2013-07-02 11:35     ` Alexandre Belloni
  2013-07-02 11:45       ` Fabio Estevam
  0 siblings, 1 reply; 17+ messages in thread
From: Alexandre Belloni @ 2013-07-02 11:35 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Shawn Guo, Jonathan Cameron, brian, linux-doc, linux-iio,
	devicetree-discuss, jimwall, Rob Herring, linux-kernel,
	Grant Likely, Rob Landley, Maxime Ripard, linux-arm-kernel,
	Lucas Stach, Marek Vasut

Hi,

On 02/07/2013 04:45, Fabio Estevam wrote:
> Are you able to use the mxs i2c controller instead of bitbang if you
> use this patch?
> http://www.spinics.net/lists/stable/msg13202.html
> 

Actually, it gets worse. I'm probably doing something wrong but I get:

mxs-i2c 8005a000.i2c: Failed to get PIO reg. write descriptor.

That is the one line 243. I'm open to any suggestion.

BTW, I also tested PIO mode with and without touchscreen support as that
seemed to toggle the issue on your side. It is also worse than what it
was in 3.9. Trying to read the nau7802 ADC never returns.

The excerpt from my DT:

i2c1: i2c@8005a000 {
	pinctrl-names = "default";
	pinctrl-0 = <&i2c1_pins_a>;
	status = "okay";
};

i2cmux {
	compatible = "i2c-mux-gpio";
	#address-cells = <1>;
	#size-cells = <0>;
	pinctrl-names = "default";
	pinctrl-0 = <&i2cmux_pins_cfa10049>;
	mux-gpios = <&gpio1 22 0 &gpio1 23 0>;
	i2c-parent = <&i2c1>;

	i2c@0 {
		reg = <0>;
		#address-cells = <1>;
		#size-cells = <0>;

		adc0: nau7802@2a {
			compatible = "nuvoton,nau7802";
			reg = <0x2a>;
			nuvoton,vldo = <3000>;
		};
	};

[...]


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCHv3 2/3] ARM: mxs: cfa10049: Switch bus i2c1 to bitbanging
  2013-07-02 11:35     ` Alexandre Belloni
@ 2013-07-02 11:45       ` Fabio Estevam
  2013-07-02 11:50         ` Alexandre Belloni
  0 siblings, 1 reply; 17+ messages in thread
From: Fabio Estevam @ 2013-07-02 11:45 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Shawn Guo, Jonathan Cameron, brian, linux-doc, linux-iio,
	devicetree-discuss, jimwall, Rob Herring, linux-kernel,
	Grant Likely, Rob Landley, Maxime Ripard, linux-arm-kernel,
	Lucas Stach, Marek Vasut

On Tue, Jul 2, 2013 at 8:35 AM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:

> mxs-i2c 8005a000.i2c: Failed to get PIO reg. write descriptor.
>
> That is the one line 243. I'm open to any suggestion.
>
> BTW, I also tested PIO mode with and without touchscreen support as that
> seemed to toggle the issue on your side. It is also worse than what it
> was in 3.9. Trying to read the nau7802 ADC never returns.
>
> The excerpt from my DT:
>
> i2c1: i2c@8005a000 {
>         pinctrl-names = "default";
>         pinctrl-0 = <&i2c1_pins_a>;
>         status = "okay";
> };
>
> i2cmux {
>         compatible = "i2c-mux-gpio";
>         #address-cells = <1>;
>         #size-cells = <0>;
>         pinctrl-names = "default";
>         pinctrl-0 = <&i2cmux_pins_cfa10049>;
>         mux-gpios = <&gpio1 22 0 &gpio1 23 0>;
>         i2c-parent = <&i2c1>;
>
>         i2c@0 {
>                 reg = <0>;

Shouldn't this be

         i2c@1 {
                 reg = <1>; ?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCHv3 2/3] ARM: mxs: cfa10049: Switch bus i2c1 to bitbanging
  2013-07-02 11:45       ` Fabio Estevam
@ 2013-07-02 11:50         ` Alexandre Belloni
  2013-07-02 14:06           ` Alexandre Belloni
  0 siblings, 1 reply; 17+ messages in thread
From: Alexandre Belloni @ 2013-07-02 11:50 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Shawn Guo, Jonathan Cameron, brian, linux-doc, linux-iio,
	devicetree-discuss, jimwall, Rob Herring, linux-kernel,
	Grant Likely, Rob Landley, Maxime Ripard, linux-arm-kernel,
	Lucas Stach, Marek Vasut

On 02/07/2013 13:45, Fabio Estevam wrote:

> Shouldn't this be
> 
>          i2c@1 {
>                  reg = <1>; ?
> 

No, we have 4 devices on that mux and 2 pins to select the muxing.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCHv3 2/3] ARM: mxs: cfa10049: Switch bus i2c1 to bitbanging
  2013-07-02 11:50         ` Alexandre Belloni
@ 2013-07-02 14:06           ` Alexandre Belloni
  2013-07-02 16:33             ` Marek Vasut
  0 siblings, 1 reply; 17+ messages in thread
From: Alexandre Belloni @ 2013-07-02 14:06 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Shawn Guo, Jonathan Cameron, brian, linux-doc, linux-iio,
	devicetree-discuss, jimwall, Rob Herring, linux-kernel,
	Grant Likely, Rob Landley, Maxime Ripard, linux-arm-kernel,
	Lucas Stach, Marek Vasut

On 02/07/2013 13:50, Alexandre Belloni wrote:
> On 02/07/2013 13:45, Fabio Estevam wrote:
> 
>> Shouldn't this be
>>
>>          i2c@1 {
>>                  reg = <1>; ?
>>
> 
> No, we have 4 devices on that mux and 2 pins to select the muxing.
> 

OK, got it working.

So, the results:

bitbanging:

# time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw
2637
real	0m 0.09s
user	0m 0.01s
sys	0m 0.01s


i2c-mxs PIO mode:

# time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw
[   35.007650] [sched_delayed] sched: RT throttling activated
2627
real	0m 7.14s
user	0m 0.02s
sys	0m 0.01s


i2c-mxs PIO mode without LRADC:

# time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw
[   18.007432] [sched_delayed] sched: RT throttling activated
2629
real	0m 7.09s
user	0m 0.00s
sys	0m 0.03s


i2c-mxs DMA mode:

# time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw
2631
real	0m 0.12s
user	0m 0.01s
sys	0m 0.01s


It seems fine for me.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCHv3 2/3] ARM: mxs: cfa10049: Switch bus i2c1 to bitbanging
  2013-07-02 14:06           ` Alexandre Belloni
@ 2013-07-02 16:33             ` Marek Vasut
  2013-07-02 17:15               ` Alexandre Belloni
  0 siblings, 1 reply; 17+ messages in thread
From: Marek Vasut @ 2013-07-02 16:33 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Fabio Estevam, Shawn Guo, Jonathan Cameron, brian, linux-doc,
	linux-iio, devicetree-discuss, jimwall, Rob Herring,
	linux-kernel, Grant Likely, Rob Landley, Maxime Ripard,
	linux-arm-kernel, Lucas Stach

Dear Alexandre Belloni,

> On 02/07/2013 13:50, Alexandre Belloni wrote:
> > On 02/07/2013 13:45, Fabio Estevam wrote:
> >> Shouldn't this be
> >> 
> >>          i2c@1 {
> >>          
> >>                  reg = <1>; ?
> > 
> > No, we have 4 devices on that mux and 2 pins to select the muxing.
> 
> OK, got it working.
> 
> So, the results:
> 
> bitbanging:
> 
> # time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw
> 2637
> real	0m 0.09s
> user	0m 0.01s
> sys	0m 0.01s
> 
> 
> i2c-mxs PIO mode:
> 
> # time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw
> [   35.007650] [sched_delayed] sched: RT throttling activated
> 2627
> real	0m 7.14s
> user	0m 0.02s
> sys	0m 0.01s
> 
> 
> i2c-mxs PIO mode without LRADC:
> 
> # time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw
> [   18.007432] [sched_delayed] sched: RT throttling activated
> 2629
> real	0m 7.09s
> user	0m 0.00s
> sys	0m 0.03s
> 
> 
> i2c-mxs DMA mode:
> 
> # time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw
> 2631
> real	0m 0.12s
> user	0m 0.01s
> sys	0m 0.01s
> 
> 
> It seems fine for me.

I think I'm getting a little lost in these gazilions of i2c and lradc threads. 
Can we not create one thread and keep the related stuff in there instead of 
discussing it all around !?

Only one question comes to mind with this email -- what do LRADC and I2C have to 
do with each other here ?

It'd be nice if someone could summarize on what I should focus and possibly 
prepare a testcase.

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCHv3 2/3] ARM: mxs: cfa10049: Switch bus i2c1 to bitbanging
  2013-07-02 16:33             ` Marek Vasut
@ 2013-07-02 17:15               ` Alexandre Belloni
  0 siblings, 0 replies; 17+ messages in thread
From: Alexandre Belloni @ 2013-07-02 17:15 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Fabio Estevam, Shawn Guo, Jonathan Cameron, brian, linux-doc,
	linux-iio, devicetree-discuss, jimwall, Rob Herring,
	linux-kernel, Grant Likely, Rob Landley, Maxime Ripard,
	linux-arm-kernel, Lucas Stach

On 02/07/2013 18:33, Marek Vasut wrote:
> Dear Alexandre Belloni,
> 
>> On 02/07/2013 13:50, Alexandre Belloni wrote:
>>> On 02/07/2013 13:45, Fabio Estevam wrote:
>>>> Shouldn't this be
>>>>
>>>>          i2c@1 {
>>>>          
>>>>                  reg = <1>; ?
>>>
>>> No, we have 4 devices on that mux and 2 pins to select the muxing.
>>
>> OK, got it working.
>>
>> So, the results:
>>
>> bitbanging:
>>
>> # time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw
>> 2637
>> real	0m 0.09s
>> user	0m 0.01s
>> sys	0m 0.01s
>>
>>
>> i2c-mxs PIO mode:
>>
>> # time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw
>> [   35.007650] [sched_delayed] sched: RT throttling activated
>> 2627
>> real	0m 7.14s
>> user	0m 0.02s
>> sys	0m 0.01s
>>
>>
>> i2c-mxs PIO mode without LRADC:
>>
>> # time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw
>> [   18.007432] [sched_delayed] sched: RT throttling activated
>> 2629
>> real	0m 7.09s
>> user	0m 0.00s
>> sys	0m 0.03s
>>
>>
>> i2c-mxs DMA mode:
>>
>> # time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw
>> 2631
>> real	0m 0.12s
>> user	0m 0.01s
>> sys	0m 0.01s
>>
>>
>> It seems fine for me.
> 
> I think I'm getting a little lost in these gazilions of i2c and lradc threads. 
> Can we not create one thread and keep the related stuff in there instead of 
> discussing it all around !?
> 
> Only one question comes to mind with this email -- what do LRADC and I2C have to 
> do with each other here ?
> 

Yeah, sorry, I meant the lradc touchscreen support. This seemed to
trigger the issue for Fabio but as my testing shows, this is not the
case for me, I get the issue with PIO, whether the lradc touchscreen
support is activated or not.

I think Torsten is the one that investigated it the most :

http://www.spinics.net/lists/linux-i2c/msg12619.html


> It'd be nice if someone could summarize on what I should focus and possibly 
> prepare a testcase.
> 

On my setup, it happens on every i2c read that are done in PIO mode.
But, my setup may be a bit unconventional as we are using a i2c gpio muxer.

Regards,

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCHv3 1/3] iio: Add Nuvoton NAU7802 ADC driver
  2013-06-24 17:24 ` [PATCHv3 1/3] iio: Add Nuvoton NAU7802 ADC driver Alexandre Belloni
@ 2013-07-04  9:08   ` Lars-Peter Clausen
  2013-07-06 10:24     ` Jonathan Cameron
  0 siblings, 1 reply; 17+ messages in thread
From: Lars-Peter Clausen @ 2013-07-04  9:08 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Shawn Guo, Jonathan Cameron, jimwall, brian, Maxime Ripard,
	linux-iio, linux-kernel, linux-doc, linux-arm-kernel,
	devicetree-discuss, Rob Landley, Rob Herring, Grant Likely

On 06/24/2013 07:24 PM, Alexandre Belloni wrote:
> The Nuvoton NAU7802 ADC is a 24-bit 2-channels I2C ADC, with adjustable
> gain and sampling rates.
> 
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Reviewed-by: Lars-Peter Clausen <lars@metafoo.de>

One remark though. Multiline comments should be like

/*
 * foo
 * bar
 */

not

/* foo
 * bar
 */

But not need to resend the patch just for this

- Lars

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCHv3 1/3] iio: Add Nuvoton NAU7802 ADC driver
  2013-07-04  9:08   ` Lars-Peter Clausen
@ 2013-07-06 10:24     ` Jonathan Cameron
  2013-07-09 14:04       ` Alexandre Belloni
  0 siblings, 1 reply; 17+ messages in thread
From: Jonathan Cameron @ 2013-07-06 10:24 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Alexandre Belloni, Shawn Guo, Jonathan Cameron, jimwall, brian,
	Maxime Ripard, linux-iio, linux-kernel, linux-doc,
	linux-arm-kernel, devicetree-discuss, Rob Landley, Rob Herring,
	Grant Likely

On 07/04/2013 10:08 AM, Lars-Peter Clausen wrote:
> On 06/24/2013 07:24 PM, Alexandre Belloni wrote:
>> The Nuvoton NAU7802 ADC is a 24-bit 2-channels I2C ADC, with adjustable
>> gain and sampling rates.
>>
>> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> 
> Reviewed-by: Lars-Peter Clausen <lars@metafoo.de>
> 
Applied to the togreg branch of iio.git (with a few little tweaks).

There was some fuzz that required manual fixing because of another driver merge so
please check I didn't foul anything up with this or the other bits below.

There were two trivial missing unlocks on error paths in the read_raw function that
I've fixed up (Coccinelle found these for me - if you can I would always advise
running sparse, coccicheck and ideally smatch before submitting patches).

I fixed up the comments because if there is one thing I hate bothering
with it is delightful patches that just fix this stuff. Much better
to do it now given it took 20 secs ;)

> One remark though. Multiline comments should be like
> 
> /*
>  * foo
>  * bar
>  */
> 
> not
> 
> /* foo
>  * bar
>  */
> 
> But not need to resend the patch just for this
> 
> - Lars
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCHv3 2/3] ARM: mxs: cfa10049: Switch bus i2c1 to bitbanging
  2013-06-24 17:24 ` [PATCHv3 2/3] ARM: mxs: cfa10049: Switch bus i2c1 to bitbanging Alexandre Belloni
  2013-07-02  2:45   ` Fabio Estevam
@ 2013-07-06 10:26   ` Jonathan Cameron
  2013-07-07  8:47     ` Alexandre Belloni
  1 sibling, 1 reply; 17+ messages in thread
From: Jonathan Cameron @ 2013-07-06 10:26 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Shawn Guo, Jonathan Cameron, jimwall, brian, Maxime Ripard,
	linux-iio, linux-kernel, linux-doc, linux-arm-kernel,
	devicetree-discuss, Rob Landley, Rob Herring, Grant Likely

On 06/24/2013 06:24 PM, Alexandre Belloni wrote:
> From: Maxime Ripard <maxime.ripard@free-electrons.com>
> 
> The ADCs connected to this bus have been experiencing some timeout
> issues when using the iMX28 i2c controller. Switching back to bitbanging
> solves this.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>

As there are no disadvantages in taking the driver through IIO and these changes
through the appropriate arch trees, I'd not propose to take these through IIO
(even when the discussion is done) unless specifically asked to.

Jonathan
> ---
>  arch/arm/boot/dts/imx28-cfa10049.dts | 108 +++++++++++++++++++++--------------
>  1 file changed, 65 insertions(+), 43 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx28-cfa10049.dts b/arch/arm/boot/dts/imx28-cfa10049.dts
> index 05ae549..d3758c2 100644
> --- a/arch/arm/boot/dts/imx28-cfa10049.dts
> +++ b/arch/arm/boot/dts/imx28-cfa10049.dts
> @@ -139,6 +139,17 @@
>  					fsl,pull-up = <0>; /* 0 will enable the keeper */
>  				};
>  
> +				i2c1_pins_cfa10049: i2c1@0 {
> +					reg = <0>;
> +					fsl,pinmux-ids = <
> +						0x3103 /* MX28_PAD_PWM0__GPIO */
> +						0x3113 /* MX28_PAD_PWM1__I2C1_SDA */
> +					>;
> +					fsl,drive-strength = <1>;
> +					fsl,voltage = <1>;
> +					fsl,pull-up = <1>;
> +				};
> +
>  				fiq_pins_cfa10049: fiq@0 {
>  					reg = <0>;
>  					fsl,pinmux-ids = <
> @@ -199,49 +210,6 @@
>  				status = "okay";
>  			};
>  
> -			i2c1: i2c@8005a000 {
> -				pinctrl-names = "default";
> -				pinctrl-0 = <&i2c1_pins_a>;
> -				status = "okay";
> -			};
> -
> -			i2cmux {
> -				compatible = "i2c-mux-gpio";
> -				#address-cells = <1>;
> -				#size-cells = <0>;
> -				mux-gpios = <&gpio1 22 0 &gpio1 23 0>;
> -				i2c-parent = <&i2c1>;
> -
> -				i2c@0 {
> -					reg = <0>;
> -				};
> -
> -				i2c@1 {
> -					reg = <1>;
> -				};
> -
> -				i2c@2 {
> -					reg = <2>;
> -				};
> -
> -				i2c@3 {
> -					reg = <3>;
> -					#address-cells = <1>;
> -					#size-cells = <0>;
> -
> -					pca9555: pca9555@20 {
> -						compatible = "nxp,pca9555";
> -						interrupt-parent = <&gpio2>;
> -						interrupts = <19 0x2>;
> -						gpio-controller;
> -						#gpio-cells = <2>;
> -						interrupt-controller;
> -						#interrupt-cells = <2>;
> -						reg = <0x20>;
> -					};
> -				};
> -			};
> -
>  			usbphy1: usbphy@8007e000 {
>  				status = "okay";
>  			};
> @@ -366,6 +334,60 @@
>  		rotary-encoder,relative-axis;
>  	};
>  
> +	i2c1gpio: i2c@0 {
> +		compatible = "i2c-gpio";
> +		pinctrl-0 = <&i2c1_pins_cfa10049>;
> +		pinctrl-names = "default";
> +		gpios = <
> +			&gpio3 17 0 /* sda */
> +			&gpio3 16 0 /* scl */
> +			 >;
> +		i2c-gpio,delay-us = <2>;	/* ~100 kHz */
> +	};
> +
> +	i2cmux {
> +		compatible = "i2c-mux-gpio";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		mux-gpios = <&gpio1 22 0 &gpio1 23 0>;
> +		i2c-parent = <&i2c1gpio>;
> +
> +		i2c@0 {
> +			reg = <0>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +		};
> +
> +		i2c@1 {
> +			reg = <1>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +		};
> +
> +		i2c@2 {
> +			reg = <2>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +		};
> +
> +		i2c@3 {
> +			reg = <3>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			pca9555: pca9555@20 {
> +				compatible = "nxp,pca9555";
> +				interrupt-parent = <&gpio2>;
> +				interrupts = <19 0x2>;
> +				gpio-controller;
> +				#gpio-cells = <2>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				reg = <0x20>;
> +			};
> +		};
> +	};
> +
>  	backlight {
>  		compatible = "pwm-backlight";
>  		pwms = <&pwm 3 5000000>;
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCHv3 2/3] ARM: mxs: cfa10049: Switch bus i2c1 to bitbanging
  2013-07-06 10:26   ` Jonathan Cameron
@ 2013-07-07  8:47     ` Alexandre Belloni
  0 siblings, 0 replies; 17+ messages in thread
From: Alexandre Belloni @ 2013-07-07  8:47 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Shawn Guo, Jonathan Cameron, jimwall, brian, Maxime Ripard,
	linux-iio, linux-kernel, linux-doc, linux-arm-kernel,
	devicetree-discuss, Rob Landley, Rob Herring, Grant Likely

On 06/07/2013 12:26, Jonathan Cameron wrote:
> On 06/24/2013 06:24 PM, Alexandre Belloni wrote:
>> From: Maxime Ripard <maxime.ripard@free-electrons.com>
>>
>> The ADCs connected to this bus have been experiencing some timeout
>> issues when using the iMX28 i2c controller. Switching back to bitbanging
>> solves this.
>>
>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> 
> As there are no disadvantages in taking the driver through IIO and these changes
> through the appropriate arch trees, I'd not propose to take these through IIO
> (even when the discussion is done) unless specifically asked to.
> 

Sure, especially since, we may finally not move to i2c bitbanging. I'll
probably resend a new version of those patches separately.

Shawn, please wait for the i2c-mxs PIO/DMA mode discussion to end before
applying. If possible, I would prefer not using bitbanging.

Regards,


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCHv3 1/3] iio: Add Nuvoton NAU7802 ADC driver
  2013-07-06 10:24     ` Jonathan Cameron
@ 2013-07-09 14:04       ` Alexandre Belloni
  2013-07-09 17:43         ` Jonathan Cameron
  0 siblings, 1 reply; 17+ messages in thread
From: Alexandre Belloni @ 2013-07-09 14:04 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Shawn Guo, Jonathan Cameron, jimwall, brian,
	Maxime Ripard, linux-iio, linux-kernel, linux-doc,
	linux-arm-kernel, devicetree-discuss, Rob Landley, Rob Herring,
	Grant Likely

Hi Jonathan,

I don't actually see it in iio.git on kernel.org, am I doing something
wrong ?

On 06/07/2013 12:24, Jonathan Cameron wrote:
> On 07/04/2013 10:08 AM, Lars-Peter Clausen wrote:
>> On 06/24/2013 07:24 PM, Alexandre Belloni wrote:
>>> The Nuvoton NAU7802 ADC is a 24-bit 2-channels I2C ADC, with adjustable
>>> gain and sampling rates.
>>>
>>> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> Reviewed-by: Lars-Peter Clausen <lars@metafoo.de>
>>
> Applied to the togreg branch of iio.git (with a few little tweaks).
>
> There was some fuzz that required manual fixing because of another driver merge so
> please check I didn't foul anything up with this or the other bits below.
>
> There were two trivial missing unlocks on error paths in the read_raw function that
> I've fixed up (Coccinelle found these for me - if you can I would always advise
> running sparse, coccicheck and ideally smatch before submitting patches).
>
> I fixed up the comments because if there is one thing I hate bothering
> with it is delightful patches that just fix this stuff. Much better
> to do it now given it took 20 secs ;)
>
>> One remark though. Multiline comments should be like
>>
>> /*
>>  * foo
>>  * bar
>>  */
>>
>> not
>>
>> /* foo
>>  * bar
>>  */
>>
>> But not need to resend the patch just for this
>>
>> - Lars
>>


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCHv3 1/3] iio: Add Nuvoton NAU7802 ADC driver
  2013-07-09 14:04       ` Alexandre Belloni
@ 2013-07-09 17:43         ` Jonathan Cameron
  0 siblings, 0 replies; 17+ messages in thread
From: Jonathan Cameron @ 2013-07-09 17:43 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Lars-Peter Clausen, Shawn Guo, Jonathan Cameron, jimwall, brian,
	Maxime Ripard, linux-iio, linux-kernel, linux-doc,
	linux-arm-kernel, devicetree-discuss, Rob Landley, Rob Herring,
	Grant Likely

Sorry, looks like I forgot to push the tree out.
Anyhow should be there shortly.

On 07/09/2013 03:04 PM, Alexandre Belloni wrote:
> Hi Jonathan,
> 
> I don't actually see it in iio.git on kernel.org, am I doing something
> wrong ?
> 
> On 06/07/2013 12:24, Jonathan Cameron wrote:
>> On 07/04/2013 10:08 AM, Lars-Peter Clausen wrote:
>>> On 06/24/2013 07:24 PM, Alexandre Belloni wrote:
>>>> The Nuvoton NAU7802 ADC is a 24-bit 2-channels I2C ADC, with adjustable
>>>> gain and sampling rates.
>>>>
>>>> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>>> Reviewed-by: Lars-Peter Clausen <lars@metafoo.de>
>>>
>> Applied to the togreg branch of iio.git (with a few little tweaks).
>>
>> There was some fuzz that required manual fixing because of another driver merge so
>> please check I didn't foul anything up with this or the other bits below.
>>
>> There were two trivial missing unlocks on error paths in the read_raw function that
>> I've fixed up (Coccinelle found these for me - if you can I would always advise
>> running sparse, coccicheck and ideally smatch before submitting patches).
>>
>> I fixed up the comments because if there is one thing I hate bothering
>> with it is delightful patches that just fix this stuff. Much better
>> to do it now given it took 20 secs ;)
>>
>>> One remark though. Multiline comments should be like
>>>
>>> /*
>>>  * foo
>>>  * bar
>>>  */
>>>
>>> not
>>>
>>> /* foo
>>>  * bar
>>>  */
>>>
>>> But not need to resend the patch just for this
>>>
>>> - Lars
>>>
> 
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2013-07-09 17:43 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-24 17:24 [PATCHv3 0/3] Add support for the Nuvoton NAU7802 ADC to the cfa10049 Alexandre Belloni
2013-06-24 17:24 ` [PATCHv3 1/3] iio: Add Nuvoton NAU7802 ADC driver Alexandre Belloni
2013-07-04  9:08   ` Lars-Peter Clausen
2013-07-06 10:24     ` Jonathan Cameron
2013-07-09 14:04       ` Alexandre Belloni
2013-07-09 17:43         ` Jonathan Cameron
2013-06-24 17:24 ` [PATCHv3 2/3] ARM: mxs: cfa10049: Switch bus i2c1 to bitbanging Alexandre Belloni
2013-07-02  2:45   ` Fabio Estevam
2013-07-02 11:35     ` Alexandre Belloni
2013-07-02 11:45       ` Fabio Estevam
2013-07-02 11:50         ` Alexandre Belloni
2013-07-02 14:06           ` Alexandre Belloni
2013-07-02 16:33             ` Marek Vasut
2013-07-02 17:15               ` Alexandre Belloni
2013-07-06 10:26   ` Jonathan Cameron
2013-07-07  8:47     ` Alexandre Belloni
2013-06-24 17:24 ` [PATCHv3 3/3] ARM: mxs: cfa10049: Add NAU7802 ADCs to the device tree Alexandre Belloni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).