Re: Blockers on IIO usage of regmap.

Re: Blockers on IIO usage of regmap.

[Jonathan Cameron]

On 09/06/11 22:13, J.I. Cameron wrote:
> On Sep 6 2011, Mark Brown wrote:
> 
>> On Tue, Sep 06, 2011 at 12:14:48PM +0100, Jonathan Cameron wrote:
>>
>>> Just a quick heads up that the big blocker for
>>> us making more use of regmap is lack of default
>>> control of cs_change for spi buses.  That leads to
>>
>> Looking at what you've got here there's nothing interesting with /CS
>> here, it's all about the data formatting.  Certainly you've not
>> mentioned anything odd with /CS in any of the code.
> Sorry, email got somewhat mashed up in editing (pesky
> dentist appointment in the middle!).
> 
> you are quite correct. Two completely different issues.
> The spi one needs addressing in the spi core, so is a
> topic for another thread entirely (I have proposal but
> need to test that thoroughly before sending out). Here
> I was simply going to say there are issues with lack of
> control of that except on a individual transfer basis.
> 
> Anyhow, on to the actual email topic...
>>
>>> TX  Add0...Add7 XXXXXXXXXXX  XXXXXXXXXXX  XXXXXXXX
>>> RX  XXXXXXXXXXX Da0.....Da7  Db0.....Db7  etc
>>
>> This is the standard one that most things do so is already supported.
>>
>>> TX  Add0...Add7 XXXXXXXXXXX  XXXXXXXXXXX  XXXXXXXXXX
>>> RX  XXXXXXXXXXX XXXXXXXXXXX  Da0.....Da7  Db0....Db7
>>
>> This is just inserting a delay so should be trivial to implement and
>> doesn't even look terribly SPI specific, just set a flag and it should
>> be fine.  All we need here is someone to actually implement it.  I guess
>> the delay will be OK for any read?
> 
> Yup. It tends to be required for all reads on a given device. Note it
> isn't typically required for writes on the devices I have but I can
> see some devices may need a 'gap' for that as well.  There is a slight
> kicker which is 'magic' address values that trigger a read of all
> registers whilst other addresses don't work for burst reads.

Hohum. Every time I look at these devices things turn out to be more
complex.  Given we have a whole heap of similar devices would it be valid
to define our own local regmap-spi-adis.c file to handle the variant?

To explain what we actually have (with all the cs line changes as well)

Writes are all 8 bit 

CS -_______________________-
TX  Ada0...Ada7  Da0....Da7
RX XXXXXXXXXXXXXXXXXXXXXXXXX

Reads are 16 bit with either of the two 8 bit register addresses given the same value

CS -______________________-_____________________-
TX  Ada0....Ada7 XXXXXXXX
RX  XXXXXXXXXXXXXXXXXXXXX  Da0...Da7  Db0....Db7  

Can interpret Da0...Da7 and Db0....Db7 as single 16 bit register and consider this device
to just have a weird write method and normal read.  Might be easier.  I'll define Ax<n> as
16 bit address for the burst read.

Bursts are mostly

CS -_________________________________________________________________________________________-
TX  Magic_address (3e) 000000000   XXXXXXXXXXXXX XXXXXXXXXXXXXXXXXXXXXXXXXX  ~~ XXXXXXXXXXXX
RX  XXXXXXXXXXXXXXXXXXXXXXXXXXXX   Axa0....Axa15 Axb0....Axb15 Axc0...Axc15  ~~ Axz0...Axz15

There is a tedious set of variant (old) where burst mode isn't supported and the duplex method
given below is needed.  Note burst reading only applies to a particular set of registers and
crucially can run at a higher spi rate than normal reading (sometimes - depends on sampling
rate).

I would just ignore regmap for these devices, except they otherwise fit well (lots of static
registers)

Perhaps the burst mode thing is better handled by just providing a hook to allow data to be pushed
into regmap (from 'magic' sources), but the weird write read combination looks to me like something
that makes sense to have in regmap (be it as another bus variant).


Just for reference, this setup applies to

iio/imu/adis16400 (my test part is a adis16350)
iio/accel/adis16201 (no burst mode).
iio/accel/adis16203 (no burst mode).
iio/accel/adis16204 (no burst mode).
iio/accel/adis16209 (no burst mode).
iio/accel/adis16220 (no burst mode - weird device ;)
iio/accel/adis16240 (no burst mode)
iio/gyro/adis16260 (no burst mode)

It's rather hacky and VERY restricted, but the series that follows this illustrates what I mean.
It's a fun one to actually apply though as you'll need my iio-blue.git tree, Mark's regmap
tree and the additional write mask patches.   Oh and it includes a first pass at adding cs_change
global control to spi devices. Patches should be as replies to this and are definitely an RFC
where we can pick and choose what we want... 

 
>>
>>> TX  Ada0...Ada7  Adb1...Adb7 Adc1...Adc7  etc
>>> RX  XXXXXXXXXXX  Da0.....Da7 Db0.....Db7  etc
>>
>> This one is much more of a contortion to implement - it's pretty SPI
>> specific to stream the register addresses and it's really not what the
>> system is set up for.  On the other hand if you can come up with a
>> taseful way to implement it then I don't see a problem.
> I'll have a play and get back to you. Another option might be a hook
> into regmap to push in register values from reads that regmap itself
> doesn't know how to do.  That way the debugfs stuff is useful and things
> aren't 'too' clunky + caching etc will work.  This sort of interleaved
> read write is vital for performance on some devices that otherwise fit
> perfectly into regmap so may be worth the effort!
> 
> It may seem odd, but it might also be handy to have a regmap_get_bus
> hook in regmap for these odd cases as it saves on carrying a bonus
> pointer to the underlying bus around the code (often also needed to
> get you access to stuff that isn't really part of the bus at all
> such as auxiliary irq numbers).
>>
>>> So basically we need some bus specific 'mode' hook somewhere.
>>> Given we have separate init functions for the buses could we
>>> add a struct regmap_spi_config to the parameter list?
>>
>> Or just embed stuff into the main config structure at least for the
>> delay.  Nothing there seems too outrageous, and the delay sounds like
>> something someone might do on other buses (the delay will be to allow
>> time for the ADCs or whatever to set up).
> Delay indeed is general enough for this.  Might need to think a bit
> on the naming so it's clear it isn't a time value, but rather a
> case of 'don't care' data sitting before what you actually requested.
> 
> I'll bash out a patch once I've sorted the CS stuff (as my test part
> adis16350 needs that before I can use regmap for it at all).
> 
> Thanks
> 
> Jonathan

[RFC PATCH 0/6] Using regmap with ADIS devices.

[Jonathan Cameron]

Aim - allow all the advantages of regmap to work with the 'unusual'
spi interfaces of Analog Devices ADIS parts.

Missing - means of feeding results of burst reads back into regmap.

The spi change clearly needs to be circulated much more widely.
I'll do that at a later date.

It 'might' be possible to make regmap understand the non symmetric
nature of these registers (reads are 16 bit, writes 8 ;) but I
couldn't really see how to do it cleanly - hence this proposal.

Anyhow, read and poke fun at it.  It's not even vaguely cleaned up.
I want to get suggestions on how to do it better before I waste time
on doing that.

To sumarise main trick.  Treat all registers as 16 bit and mess
with the register addresses in the write function to get it
to split them in two.

Note I've halved all register addresses in the driver as that will
work better for caching in regmap at a later date.

Jonathan



Jonathan Cameron (6):
  SPI: add ability to say we want a cs change after every transfer.
  regmap: Add a magic bus type to handle quirks of analog devices ADIS
    sensors.
  staging:iio:imu: adis16400 partial conversion to regmap.
  regmap-spi-adi + staging:iio:imu:adis16400 halve register addresses
  regmap-spi-adi generalize regmap_spi_read.
  staging:iio:imu:adis16400 make use of regmap bulk read capabilities

 drivers/base/regmap/Kconfig              |    5 +-
 drivers/base/regmap/Makefile             |    1 +
 drivers/base/regmap/regmap-spi-adi.c     |   93 +++++++++
 drivers/spi/spi.c                        |    8 +
 drivers/staging/iio/imu/Kconfig          |    1 +
 drivers/staging/iio/imu/adis16400.h      |  100 +++++-----
 drivers/staging/iio/imu/adis16400_core.c |  300 ++++++++++++++++++-----------
 drivers/staging/iio/imu/adis16400_ring.c |   69 ++------
 include/linux/regmap.h                   |    2 +
 include/linux/spi/spi.h                  |    2 +
 10 files changed, 360 insertions(+), 221 deletions(-)
 create mode 100644 drivers/base/regmap/regmap-spi-adi.c

-- 
1.7.3.4

[PATCH 1/6] SPI: add ability to say we want a cs change after every transfer.

[Jonathan Cameron]

This allows a number of drivers to make use of utility functions
such spi_write_then_read as well as making use of regmap possible.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/spi/spi.c       |    8 ++++++++
 include/linux/spi/spi.h |    2 ++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 4d1b9f5..bee8aee 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -781,6 +781,14 @@ static int __spi_async(struct spi_device *spi, struct spi_message *message)
 		}
 	}
 
+	if (spi->cs_between_transfers) {
+		struct spi_transfer *xfer;
+		list_for_each_entry(xfer, &message->transfers, transfer_list) {
+			if (!list_is_last(&xfer->transfer_list, &message->transfers))
+				xfer->cs_change = 1;
+		}
+	}
+	    
 	message->spi = spi;
 	message->status = -EINPROGRESS;
 	return master->transfer(spi, message);
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index bb4f5fb..f1e378d 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -90,6 +90,8 @@ struct spi_device {
 	void			*controller_data;
 	char			modalias[SPI_NAME_SIZE];
 
+  unsigned cs_between_transfers:1;
+
 	/*
 	 * likely need more hooks for more protocol options affecting how
 	 * the controller talks to each chip, like:
-- 
1.7.3.4

[PATCH 2/6] regmap: Add a magic bus type to handle quirks of analog devices ADIS sensors.

[Jonathan Cameron]

These devices look like 8 bit registers for writes and 16 bit registers for
reads.  As you might imagine this causes some 'issues' hence this regmap
bus implementation claims they are always 16bit and does the mangling to
make the writes work.
---
 drivers/base/regmap/Kconfig          |    5 ++-
 drivers/base/regmap/Makefile         |    1 +
 drivers/base/regmap/regmap-spi-adi.c |   70 ++++++++++++++++++++++++++++++++++
 include/linux/regmap.h               |    2 +
 4 files changed, 77 insertions(+), 1 deletions(-)

diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
index fabbf6c..e4991fe 100644
--- a/drivers/base/regmap/Kconfig
+++ b/drivers/base/regmap/Kconfig
@@ -3,7 +3,7 @@
 # subsystems should select the appropriate symbols.
 
 config REGMAP
-	default y if (REGMAP_I2C || REGMAP_SPI)
+	default y if (REGMAP_I2C || REGMAP_SPI || REGMAP_SPI_ADI)
 	bool
 
 config REGMAP_I2C
@@ -11,3 +11,6 @@ config REGMAP_I2C
 
 config REGMAP_SPI
 	tristate
+
+config REGMAP_SPI_ADI
+	tristate
diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile
index 057c13f..9e71bf8 100644
--- a/drivers/base/regmap/Makefile
+++ b/drivers/base/regmap/Makefile
@@ -2,3 +2,4 @@ obj-$(CONFIG_REGMAP) += regmap.o
 obj-$(CONFIG_DEBUG_FS) += regmap-debugfs.o
 obj-$(CONFIG_REGMAP_I2C) += regmap-i2c.o
 obj-$(CONFIG_REGMAP_SPI) += regmap-spi.o
+obj-$(CONFIG_REGMAP_SPI_ADI) += regmap-spi-adi.o
diff --git a/drivers/base/regmap/regmap-spi-adi.c b/drivers/base/regmap/regmap-spi-adi.c
new file mode 100644
index 0000000..a98c000
--- /dev/null
+++ b/drivers/base/regmap/regmap-spi-adi.c
@@ -0,0 +1,70 @@
+/*
+ * Register map access API - SPI support
+ *
+ * Copyright 2011 Wolfson Microelectronics plc
+ *
+ * Author: Mark Brown <broonie@opensource.wolfsonmicro.com>
+ *
+ * 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/regmap.h>
+#include <linux/spi/spi.h>
+#include <linux/init.h>
+#include <linux/module.h>
+
+static int regmap_spi_write(struct device *dev, const void *data, size_t count)
+{
+/* Now this only works for 8 bit addresss 16 bit register first byte of data
+ * is the lower address, second two the value */
+	struct spi_device *spi = to_spi_device(dev);
+	int ret;
+	u8 *rawdata = (u8 *)data;
+
+	BUG_ON(count != 3);
+	/* Fiddling needed as the value is bigendian */
+	rawdata[0] += 1;
+	ret = spi_write(spi, data, 2);
+	if (ret < 0)
+		return ret;
+
+	rawdata[1] = rawdata[0] - 1;
+	ret = spi_write(spi, data + 1, 2);
+	return ret;
+}
+
+static int regmap_spi_read(struct device *dev,
+			   const void *reg, size_t reg_size,
+			   void *val, size_t val_size)
+{
+	struct spi_device *spi = to_spi_device(dev);
+
+	BUG_ON(reg_size != 1 || val_size != 2);
+
+	return spi_write_then_read(spi, reg, 2, val, val_size);
+}
+
+static struct regmap_bus regmap_spi_adi = {
+	.write = regmap_spi_write,
+	.read = regmap_spi_read,
+};
+
+/**
+ * regmap_init_spi_adi(): Initialise register map
+ *
+ * @spi: Device that will be interacted with
+ * @config: Configuration for register map
+ *
+ * The return value will be an ERR_PTR() on error or a valid pointer to
+ * a struct regmap.
+ */
+struct regmap *regmap_init_spi_adi(struct spi_device *spi,
+				   const struct regmap_config *config)
+{
+	return regmap_init(&spi->dev, &regmap_spi_adi, config);
+}
+EXPORT_SYMBOL_GPL(regmap_init_spi_adi);
+
+MODULE_LICENSE("GPL");
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 18d4afa..a827c30 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -111,6 +111,8 @@ struct regmap *regmap_init_i2c(struct i2c_client *i2c,
 			       const struct regmap_config *config);
 struct regmap *regmap_init_spi(struct spi_device *dev,
 			       const struct regmap_config *config);
+struct regmap *regmap_init_spi_adi(struct spi_device *dev,
+			       const struct regmap_config *config);
 
 void regmap_exit(struct regmap *map);
 int regmap_write(struct regmap *map, unsigned int reg, unsigned int val);
-- 
1.7.3.4

[PATCH 3/6] staging:iio:imu: adis16400 partial conversion to regmap.

[Jonathan Cameron]

This requires the regmap-spi-adi regmap bus to deal with
complex read / write combinations.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/staging/iio/imu/Kconfig          |    1 +
 drivers/staging/iio/imu/adis16400.h      |    2 +
 drivers/staging/iio/imu/adis16400_core.c |  300 ++++++++++++++++++-----------
 drivers/staging/iio/imu/adis16400_ring.c |    2 +-
 4 files changed, 190 insertions(+), 115 deletions(-)

diff --git a/drivers/staging/iio/imu/Kconfig b/drivers/staging/iio/imu/Kconfig
index e0e0144..7dfaa43 100644
--- a/drivers/staging/iio/imu/Kconfig
+++ b/drivers/staging/iio/imu/Kconfig
@@ -6,6 +6,7 @@ comment "Inertial measurement units"
 config ADIS16400
 	tristate "Analog Devices ADIS16400 and similar IMU SPI driver"
 	depends on SPI
+	select REGMAP_SPI_ADI
 	select IIO_SW_RING if IIO_RING_BUFFER
 	select IIO_TRIGGER if IIO_RING_BUFFER
 	help
diff --git a/drivers/staging/iio/imu/adis16400.h b/drivers/staging/iio/imu/adis16400.h
index 1f8f0c6..0bf1fd9 100644
--- a/drivers/staging/iio/imu/adis16400.h
+++ b/drivers/staging/iio/imu/adis16400.h
@@ -141,6 +141,7 @@ struct adis16400_chip_info {
 	unsigned long default_scan_mask;
 };
 
+struct regmap;
 /**
  * struct adis16400_state - device instance specific data
  * @us:			actual spi_device
@@ -150,6 +151,7 @@ struct adis16400_chip_info {
  * @buf_lock:		mutex to protect tx and rx
  **/
 struct adis16400_state {
+	struct regmap *regmap;
 	struct spi_device		*us;
 	struct iio_trigger		*trig;
 	struct mutex			buf_lock;
diff --git a/drivers/staging/iio/imu/adis16400_core.c b/drivers/staging/iio/imu/adis16400_core.c
index b6d824f..b4be380 100644
--- a/drivers/staging/iio/imu/adis16400_core.c
+++ b/drivers/staging/iio/imu/adis16400_core.c
@@ -25,6 +25,8 @@
 #include <linux/sysfs.h>
 #include <linux/list.h>
 #include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/err.h>
 
 #include "../iio.h"
 #include "../sysfs.h"
@@ -42,28 +44,6 @@ enum adis16400_chip_variant {
 	ADIS16400,
 };
 
-/**
- * adis16400_spi_write_reg_8() - write single byte to a register
- * @dev: device associated with child of actual device (iio_dev or iio_trig)
- * @reg_address: the address of the register to be written
- * @val: the value to write
- */
-static int adis16400_spi_write_reg_8(struct iio_dev *indio_dev,
-				     u8 reg_address,
-				     u8 val)
-{
-	int ret;
-	struct adis16400_state *st = iio_priv(indio_dev);
-
-	mutex_lock(&st->buf_lock);
-	st->tx[0] = ADIS16400_WRITE_REG(reg_address);
-	st->tx[1] = val;
-
-	ret = spi_write(st->us, st->tx, 2);
-	mutex_unlock(&st->buf_lock);
-
-	return ret;
-}
 
 /**
  * adis16400_spi_write_reg_16() - write 2 bytes to a pair of registers
@@ -71,43 +51,14 @@ static int adis16400_spi_write_reg_8(struct iio_dev *indio_dev,
  * @reg_address: the address of the lower of the two registers. Second register
  *               is assumed to have address one greater.
  * @val: value to be written
- *
- * At the moment the spi framework doesn't allow global setting of cs_change.
- * This means that use cannot be made of spi_write.
  */
 static int adis16400_spi_write_reg_16(struct iio_dev *indio_dev,
-		u8 lower_reg_address,
-		u16 value)
+				      u8 lower_reg_address,
+				      u16 value)
 {
-	int ret;
-	struct spi_message msg;
 	struct adis16400_state *st = iio_priv(indio_dev);
-	struct spi_transfer xfers[] = {
-		{
-			.tx_buf = st->tx,
-			.bits_per_word = 8,
-			.len = 2,
-			.cs_change = 1,
-		}, {
-			.tx_buf = st->tx + 2,
-			.bits_per_word = 8,
-			.len = 2,
-		},
-	};
-
-	mutex_lock(&st->buf_lock);
-	st->tx[0] = ADIS16400_WRITE_REG(lower_reg_address);
-	st->tx[1] = value & 0xFF;
-	st->tx[2] = ADIS16400_WRITE_REG(lower_reg_address + 1);
-	st->tx[3] = (value >> 8) & 0xFF;
 
-	spi_message_init(&msg);
-	spi_message_add_tail(&xfers[0], &msg);
-	spi_message_add_tail(&xfers[1], &msg);
-	ret = spi_sync(st->us, &msg);
-	mutex_unlock(&st->buf_lock);
-
-	return ret;
+	return regmap_write(st->regmap, lower_reg_address, value);
 }
 
 /**
@@ -116,49 +67,22 @@ static int adis16400_spi_write_reg_16(struct iio_dev *indio_dev,
  * @reg_address: the address of the lower of the two registers. Second register
  *               is assumed to have address one greater.
  * @val: somewhere to pass back the value read
- *
- * At the moment the spi framework doesn't allow global setting of cs_change.
- * This means that use cannot be made of spi_read.
  **/
 static int adis16400_spi_read_reg_16(struct iio_dev *indio_dev,
-		u8 lower_reg_address,
-		u16 *val)
+				     u8 lower_reg_address,
+				     u16 *val)
 {
-	struct spi_message msg;
 	struct adis16400_state *st = iio_priv(indio_dev);
 	int ret;
-	struct spi_transfer xfers[] = {
-		{
-			.tx_buf = st->tx,
-			.bits_per_word = 8,
-			.len = 2,
-			.cs_change = 1,
-		}, {
-			.rx_buf = st->rx,
-			.bits_per_word = 8,
-			.len = 2,
-		},
-	};
+	unsigned int value;
 
-	mutex_lock(&st->buf_lock);
-	st->tx[0] = ADIS16400_READ_REG(lower_reg_address);
-	st->tx[1] = 0;
-
-	spi_message_init(&msg);
-	spi_message_add_tail(&xfers[0], &msg);
-	spi_message_add_tail(&xfers[1], &msg);
-	ret = spi_sync(st->us, &msg);
-	if (ret) {
-		dev_err(&st->us->dev,
-			"problem when reading 16 bit register 0x%02X",
-			lower_reg_address);
-		goto error_ret;
-	}
-	*val = (st->rx[0] << 8) | st->rx[1];
+	ret = regmap_read(st->regmap, lower_reg_address, &value);
+	if (ret < 0)
+		return ret;
 
-error_ret:
-	mutex_unlock(&st->buf_lock);
-	return ret;
+	*val = value;
+
+	return 0;
 }
 
 static ssize_t adis16400_read_frequency(struct device *dev,
@@ -172,7 +96,7 @@ static ssize_t adis16400_read_frequency(struct device *dev,
 	ret = adis16400_spi_read_reg_16(indio_dev,
 			ADIS16400_SMPL_PRD,
 			&t);
-	if (ret)
+	if (ret < 0)
 		return ret;
 	sps =  (t & ADIS16400_SMPL_PRD_TIME_BASE) ? 53 : 1638;
 	sps /= (t & ADIS16400_SMPL_PRD_DIV_MASK) + 1;
@@ -192,7 +116,7 @@ static ssize_t adis16400_write_frequency(struct device *dev,
 	u8 t;
 
 	ret = strict_strtol(buf, 10, &val);
-	if (ret)
+	if (ret < 0)
 		return ret;
 
 	mutex_lock(&indio_dev->mlock);
@@ -206,22 +130,22 @@ static ssize_t adis16400_write_frequency(struct device *dev,
 	else
 		st->us->max_speed_hz = ADIS16400_SPI_FAST;
 
-	ret = adis16400_spi_write_reg_8(indio_dev,
+	ret = adis16400_spi_write_reg_16(indio_dev,
 			ADIS16400_SMPL_PRD,
 			t);
 
 	mutex_unlock(&indio_dev->mlock);
 
-	return ret ? ret : len;
+	return ret < 0 ? ret : len;
 }
 
 static int adis16400_reset(struct iio_dev *indio_dev)
 {
 	int ret;
-	ret = adis16400_spi_write_reg_8(indio_dev,
+	ret = adis16400_spi_write_reg_16(indio_dev,
 			ADIS16400_GLOB_CMD,
 			ADIS16400_GLOB_CMD_SW_RESET);
-	if (ret)
+	if (ret < 0)
 		dev_err(&indio_dev->dev, "problem resetting device");
 
 	return ret;
@@ -252,7 +176,7 @@ int adis16400_set_irq(struct iio_dev *indio_dev, bool enable)
 	u16 msc;
 
 	ret = adis16400_spi_read_reg_16(indio_dev, ADIS16400_MSC_CTRL, &msc);
-	if (ret)
+	if (ret < 0)
 		goto error_ret;
 
 	msc |= ADIS16400_MSC_CTRL_DATA_RDY_POL_HIGH;
@@ -262,8 +186,6 @@ int adis16400_set_irq(struct iio_dev *indio_dev, bool enable)
 		msc &= ~ADIS16400_MSC_CTRL_DATA_RDY_EN;
 
 	ret = adis16400_spi_write_reg_16(indio_dev, ADIS16400_MSC_CTRL, msc);
-	if (ret)
-		goto error_ret;
 
 error_ret:
 	return ret;
@@ -338,7 +260,7 @@ static int adis16400_self_test(struct iio_dev *indio_dev)
 	ret = adis16400_spi_write_reg_16(indio_dev,
 			ADIS16400_MSC_CTRL,
 			ADIS16400_MSC_CTRL_MEM_TEST);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(&indio_dev->dev, "problem starting self test");
 		goto err_ret;
 	}
@@ -362,24 +284,24 @@ static int adis16400_initial_setup(struct iio_dev *indio_dev)
 	spi_setup(st->us);
 
 	ret = adis16400_set_irq(indio_dev, false);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(&indio_dev->dev, "disable irq failed");
 		goto err_ret;
 	}
 
 	ret = adis16400_self_test(indio_dev);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(&indio_dev->dev, "self test failure");
 		goto err_ret;
 	}
 
 	ret = adis16400_check_status(indio_dev);
-	if (ret) {
+	if (ret < 0) {
 		adis16400_reset(indio_dev);
 		dev_err(&indio_dev->dev, "device not playing ball -> reset");
 		msleep(ADIS16400_STARTUP_DELAY);
 		ret = adis16400_check_status(indio_dev);
-		if (ret) {
+		if (ret < 0) {
 			dev_err(&indio_dev->dev, "giving up");
 			goto err_ret;
 		}
@@ -387,7 +309,7 @@ static int adis16400_initial_setup(struct iio_dev *indio_dev)
 	if (st->variant->flags & ADIS16400_HAS_PROD_ID) {
 		ret = adis16400_spi_read_reg_16(indio_dev,
 						ADIS16400_PRODUCT_ID, &prod_id);
-		if (ret)
+		if (ret < 0)
 			goto err_ret;
 
 		if ((prod_id & 0xF000) != st->variant->product_id)
@@ -492,7 +414,7 @@ static int adis16400_read_raw(struct iio_dev *indio_dev,
 		ret = adis16400_spi_read_reg_16(indio_dev,
 				adis16400_addresses[chan->address][0],
 				&val16);
-		if (ret) {
+		if (ret < 0) {
 			mutex_unlock(&indio_dev->mlock);
 			return ret;
 		}
@@ -539,7 +461,7 @@ static int adis16400_read_raw(struct iio_dev *indio_dev,
 				adis16400_addresses[chan->address][1],
 				&val16);
 		mutex_unlock(&indio_dev->mlock);
-		if (ret)
+		if (ret < 0)
 			return ret;
 		val16 = ((val16 & 0xFFF) << 4) >> 4;
 		*val = val16;
@@ -1016,6 +938,146 @@ static const struct iio_info adis16400_info = {
 	.attrs = &adis16400_attribute_group,
 };
 
+/* This may technically result in read attempts to undefined registers */
+static bool adis16400_reg_readable(struct device *dev,
+				   unsigned int reg)
+{
+	switch (reg) {
+	case ADIS16400_FLASH_CNT:
+	case ADIS16400_SUPPLY_OUT:
+	case ADIS16400_XGYRO_OUT:
+	case ADIS16400_YGYRO_OUT:
+	case ADIS16400_ZGYRO_OUT:
+	case ADIS16400_XACCL_OUT:
+	case ADIS16400_YACCL_OUT:
+	case ADIS16400_ZACCL_OUT:
+	case ADIS16400_XMAGN_OUT:
+	case ADIS16400_YMAGN_OUT:
+	case ADIS16400_ZMAGN_OUT:
+	case ADIS16400_TEMP_OUT:
+	case ADIS16400_AUX_ADC:
+	case ADIS16400_XGYRO_OFF:
+	case ADIS16400_YGYRO_OFF:
+	case ADIS16400_ZGYRO_OFF:
+	case ADIS16400_XACCL_OFF:
+	case ADIS16400_YACCL_OFF:
+	case ADIS16400_ZACCL_OFF:
+	case ADIS16400_XMAGN_HIF:
+	case ADIS16400_YMAGN_HIF:
+	case ADIS16400_ZMAGN_HIF:
+	case ADIS16400_XMAGN_SIF:
+	case ADIS16400_YMAGN_SIF:
+	case ADIS16400_ZMAGN_SIF:
+	case ADIS16400_GPIO_CTRL:
+	case ADIS16400_MSC_CTRL:
+	case ADIS16400_SMPL_PRD:
+	case ADIS16400_SENS_AVG:
+	case ADIS16400_DIAG_STAT:
+	case ADIS16400_ALM_MAG1:
+	case ADIS16400_ALM_MAG2:
+	case ADIS16400_ALM_SMPL1:
+	case ADIS16400_ALM_SMPL2:
+	case ADIS16400_ALM_CTRL:
+	case ADIS16400_AUX_DAC:
+	case ADIS16400_PRODUCT_ID:
+		return true;
+	default:
+		return false;
+	};
+}
+
+static bool adis16400_reg_precious(struct device *dev,
+				   unsigned int reg)
+{
+	switch (reg) {
+	case ADIS16400_SUPPLY_OUT:
+	case ADIS16400_XGYRO_OUT:
+	case ADIS16400_YGYRO_OUT:
+	case ADIS16400_ZGYRO_OUT:
+	case ADIS16400_XACCL_OUT:
+	case ADIS16400_YACCL_OUT:
+	case ADIS16400_ZACCL_OUT:
+	case ADIS16400_XMAGN_OUT:
+	case ADIS16400_YMAGN_OUT:
+	case ADIS16400_ZMAGN_OUT:
+	case ADIS16400_TEMP_OUT:
+	case ADIS16400_AUX_ADC:
+	case ADIS16400_DIAG_STAT:
+		return true;
+	default:
+		return 0;
+	}
+}
+
+static bool adis16400_reg_volatile(struct device *dev,
+				   unsigned int reg)
+{
+	switch (reg) {
+	case ADIS16400_FLASH_CNT:
+	case ADIS16400_SUPPLY_OUT:
+	case ADIS16400_XGYRO_OUT:
+	case ADIS16400_YGYRO_OUT:
+	case ADIS16400_ZGYRO_OUT:
+	case ADIS16400_XACCL_OUT:
+	case ADIS16400_YACCL_OUT:
+	case ADIS16400_ZACCL_OUT:
+	case ADIS16400_XMAGN_OUT:
+	case ADIS16400_YMAGN_OUT:
+	case ADIS16400_ZMAGN_OUT:
+	case ADIS16400_TEMP_OUT:
+	case ADIS16400_AUX_ADC:
+	case ADIS16400_DIAG_STAT:
+
+		return true;
+	default:
+		return 0;
+	}
+}
+static bool adis16400_reg_writeable(struct device *dev,
+				    unsigned int reg)
+{
+	switch (reg) {
+	case ADIS16400_XGYRO_OFF:
+	case ADIS16400_YGYRO_OFF:
+	case ADIS16400_ZGYRO_OFF:
+	case ADIS16400_XACCL_OFF:
+	case ADIS16400_YACCL_OFF:
+	case ADIS16400_ZACCL_OFF:
+	case ADIS16400_XMAGN_HIF:
+	case ADIS16400_YMAGN_HIF:
+	case ADIS16400_ZMAGN_HIF:
+	case ADIS16400_XMAGN_SIF:
+	case ADIS16400_YMAGN_SIF:
+	case ADIS16400_ZMAGN_SIF:
+	case ADIS16400_GPIO_CTRL:
+	case ADIS16400_MSC_CTRL:
+	case ADIS16400_SMPL_PRD:
+	case ADIS16400_SENS_AVG:
+	case ADIS16400_SLP_CNT:
+	case ADIS16400_GLOB_CMD:
+	case ADIS16400_ALM_MAG1:
+	case ADIS16400_ALM_MAG2:
+	case ADIS16400_ALM_SMPL1:
+	case ADIS16400_ALM_SMPL2:
+	case ADIS16400_ALM_CTRL:
+	case ADIS16400_AUX_DAC:
+		return true;
+	default:
+		return false;
+	};
+}
+static struct regmap_config adis16400_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 16,
+	.writeable_reg = &adis16400_reg_writeable,
+	.readable_reg = &adis16400_reg_readable,
+	.precious_reg = &adis16400_reg_precious,
+	.volatile_reg = &adis16400_reg_volatile,
+	.max_register = 0x56,
+	.write_flag_mask = 0x80,
+	.read_flag_mask = 0,
+};
+
 static int __devinit adis16400_probe(struct spi_device *spi)
 {
 	int ret;
@@ -1026,8 +1088,14 @@ static int __devinit adis16400_probe(struct spi_device *spi)
 		goto error_ret;
 	}
 	st = iio_priv(indio_dev);
+	st->regmap = regmap_init_spi_adi(spi, &adis16400_regmap_config);
+	if (IS_ERR(st->regmap)) {
+		ret = PTR_ERR(st->regmap);
+		goto error_free_dev;
+	}
 	/* this is only used for removal purposes */
 	spi_set_drvdata(spi, indio_dev);
+	spi->cs_between_transfers = 1;
 
 	st->us = spi;
 	mutex_init(&st->buf_lock);
@@ -1042,29 +1110,29 @@ static int __devinit adis16400_probe(struct spi_device *spi)
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
 	ret = adis16400_configure_ring(indio_dev);
-	if (ret)
-		goto error_free_dev;
+	if (ret < 0)
+		goto error_free_regmap;
 
 	ret = iio_ring_buffer_register(indio_dev,
 				       st->variant->channels,
 				       st->variant->num_channels);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(&spi->dev, "failed to initialize the ring\n");
 		goto error_unreg_ring_funcs;
 	}
 
 	if (spi->irq) {
 		ret = adis16400_probe_trigger(indio_dev);
-		if (ret)
+		if (ret < 0)
 			goto error_uninitialize_ring;
 	}
 
 	/* Get the device into a sane initial state */
 	ret = adis16400_initial_setup(indio_dev);
-	if (ret)
+	if (ret < 0)
 		goto error_remove_trigger;
 	ret = iio_device_register(indio_dev);
-	if (ret)
+	if (ret < 0)
 		goto error_remove_trigger;
 
 	return 0;
@@ -1076,6 +1144,8 @@ error_uninitialize_ring:
 	iio_ring_buffer_unregister(indio_dev);
 error_unreg_ring_funcs:
 	adis16400_unconfigure_ring(indio_dev);
+error_free_regmap:
+	regmap_exit(st->regmap);
 error_free_dev:
 	iio_free_device(indio_dev);
 error_ret:
@@ -1087,14 +1157,16 @@ static int adis16400_remove(struct spi_device *spi)
 {
 	int ret;
 	struct iio_dev *indio_dev =  spi_get_drvdata(spi);
+	struct adis16400_state *st = iio_priv(indio_dev);
 
 	ret = adis16400_stop_device(indio_dev);
-	if (ret)
+	if (ret < 0)
 		goto err_ret;
 
 	adis16400_remove_trigger(indio_dev);
 	iio_ring_buffer_unregister(indio_dev);
 	adis16400_unconfigure_ring(indio_dev);
+	regmap_exit(st->regmap);
 	iio_device_unregister(indio_dev);
 
 	return 0;
diff --git a/drivers/staging/iio/imu/adis16400_ring.c b/drivers/staging/iio/imu/adis16400_ring.c
index af25697..8345d4e 100644
--- a/drivers/staging/iio/imu/adis16400_ring.c
+++ b/drivers/staging/iio/imu/adis16400_ring.c
@@ -47,7 +47,7 @@ static int adis16400_spi_read_burst(struct device *dev, u8 *rx)
 	spi_setup(st->us);
 
 	ret = spi_sync(st->us, &msg);
-	if (ret)
+	if (ret < 0)
 		dev_err(&st->us->dev, "problem when burst reading");
 
 	st->us->max_speed_hz = old_speed_hz;
-- 
1.7.3.4

[PATCH 4/6] regmap-spi-adi + staging:iio:imu:adis16400 halve register addresses

[Jonathan Cameron]

The register addresses were even numbers only.  That makes for very bad
caching, so lets halve them all.

Nasty trick needed with the write_flag_mask being halved to make this work.
---
 drivers/base/regmap/regmap-spi-adi.c     |    8 ++-
 drivers/staging/iio/imu/adis16400.h      |   88 +++++++++++++++---------------
 drivers/staging/iio/imu/adis16400_core.c |    2 +-
 drivers/staging/iio/imu/adis16400_ring.c |   22 ++++----
 4 files changed, 61 insertions(+), 59 deletions(-)

diff --git a/drivers/base/regmap/regmap-spi-adi.c b/drivers/base/regmap/regmap-spi-adi.c
index a98c000..d4301cc 100644
--- a/drivers/base/regmap/regmap-spi-adi.c
+++ b/drivers/base/regmap/regmap-spi-adi.c
@@ -25,14 +25,13 @@ static int regmap_spi_write(struct device *dev, const void *data, size_t count)
 
 	BUG_ON(count != 3);
 	/* Fiddling needed as the value is bigendian */
-	rawdata[0] += 1;
+	rawdata[0] = (rawdata[0] << 1) + 1;
 	ret = spi_write(spi, data, 2);
 	if (ret < 0)
 		return ret;
 
 	rawdata[1] = rawdata[0] - 1;
-	ret = spi_write(spi, data + 1, 2);
-	return ret;
+	return spi_write(spi, data + 1, 2);
 }
 
 static int regmap_spi_read(struct device *dev,
@@ -40,6 +39,9 @@ static int regmap_spi_read(struct device *dev,
 			   void *val, size_t val_size)
 {
 	struct spi_device *spi = to_spi_device(dev);
+	u8 *regraw = (u8 *)reg;
+
+	regraw[0] <<= 1;
 
 	BUG_ON(reg_size != 1 || val_size != 2);
 
diff --git a/drivers/staging/iio/imu/adis16400.h b/drivers/staging/iio/imu/adis16400.h
index 0bf1fd9..919d6fa 100644
--- a/drivers/staging/iio/imu/adis16400.h
+++ b/drivers/staging/iio/imu/adis16400.h
@@ -23,58 +23,58 @@
 #define ADIS16400_READ_REG(a)    a
 #define ADIS16400_WRITE_REG(a) ((a) | 0x80)
 
-#define ADIS16400_FLASH_CNT  0x00 /* Flash memory write count */
-#define ADIS16400_SUPPLY_OUT 0x02 /* Power supply measurement */
-#define ADIS16400_XGYRO_OUT 0x04 /* X-axis gyroscope output */
-#define ADIS16400_YGYRO_OUT 0x06 /* Y-axis gyroscope output */
-#define ADIS16400_ZGYRO_OUT 0x08 /* Z-axis gyroscope output */
-#define ADIS16400_XACCL_OUT 0x0A /* X-axis accelerometer output */
-#define ADIS16400_YACCL_OUT 0x0C /* Y-axis accelerometer output */
-#define ADIS16400_ZACCL_OUT 0x0E /* Z-axis accelerometer output */
-#define ADIS16400_XMAGN_OUT 0x10 /* X-axis magnetometer measurement */
-#define ADIS16400_YMAGN_OUT 0x12 /* Y-axis magnetometer measurement */
-#define ADIS16400_ZMAGN_OUT 0x14 /* Z-axis magnetometer measurement */
-#define ADIS16400_TEMP_OUT  0x16 /* Temperature output */
-#define ADIS16400_AUX_ADC   0x18 /* Auxiliary ADC measurement */
-
-#define ADIS16350_XTEMP_OUT 0x10 /* X-axis gyroscope temperature measurement */
-#define ADIS16350_YTEMP_OUT 0x12 /* Y-axis gyroscope temperature measurement */
-#define ADIS16350_ZTEMP_OUT 0x14 /* Z-axis gyroscope temperature measurement */
-
-#define ADIS16300_PITCH_OUT 0x12 /* X axis inclinometer output measurement */
-#define ADIS16300_ROLL_OUT  0x12 /* Y axis inclinometer output measurement */
+#define ADIS16400_FLASH_CNT  (0x00 >> 1) /* Flash memory write count */
+#define ADIS16400_SUPPLY_OUT (0x02 >> 1) /* Power supply measurement */
+#define ADIS16400_XGYRO_OUT (0x04 >> 1) /* X-axis gyroscope output */
+#define ADIS16400_YGYRO_OUT (0x06 >> 1) /* Y-axis gyroscope output */
+#define ADIS16400_ZGYRO_OUT (0x08 >> 1) /* Z-axis gyroscope output */
+#define ADIS16400_XACCL_OUT (0x0A >> 1) /* X-axis accelerometer output */
+#define ADIS16400_YACCL_OUT (0x0C >> 1) /* Y-axis accelerometer output */
+#define ADIS16400_ZACCL_OUT (0x0E >> 1) /* Z-axis accelerometer output */
+#define ADIS16400_XMAGN_OUT (0x10 >> 1) /* X-axis magnetometer measurement */
+#define ADIS16400_YMAGN_OUT (0x12 >> 1) /* Y-axis magnetometer measurement */
+#define ADIS16400_ZMAGN_OUT (0x14 >> 1) /* Z-axis magnetometer measurement */
+#define ADIS16400_TEMP_OUT  (0x16 >> 1) /* Temperature output */
+#define ADIS16400_AUX_ADC   (0x18 >> 1) /* Auxiliary ADC measurement */
+
+#define ADIS16350_XTEMP_OUT (0x10 >> 1) /* X-axis gyroscope temperature measurement */
+#define ADIS16350_YTEMP_OUT (0x12 >> 1) /* Y-axis gyroscope temperature measurement */
+#define ADIS16350_ZTEMP_OUT (0x14 >> 1) /* Z-axis gyroscope temperature measurement */
+
+#define ADIS16300_PITCH_OUT (0x12 >> 1) /* X axis inclinometer output measurement */
+#define ADIS16300_ROLL_OUT  (0x12 >> 1) /* Y axis inclinometer output measurement */
 
 /* Calibration parameters */
-#define ADIS16400_XGYRO_OFF 0x1A /* X-axis gyroscope bias offset factor */
-#define ADIS16400_YGYRO_OFF 0x1C /* Y-axis gyroscope bias offset factor */
-#define ADIS16400_ZGYRO_OFF 0x1E /* Z-axis gyroscope bias offset factor */
-#define ADIS16400_XACCL_OFF 0x20 /* X-axis acceleration bias offset factor */
-#define ADIS16400_YACCL_OFF 0x22 /* Y-axis acceleration bias offset factor */
-#define ADIS16400_ZACCL_OFF 0x24 /* Z-axis acceleration bias offset factor */
-#define ADIS16400_XMAGN_HIF 0x26 /* X-axis magnetometer, hard-iron factor */
-#define ADIS16400_YMAGN_HIF 0x28 /* Y-axis magnetometer, hard-iron factor */
-#define ADIS16400_ZMAGN_HIF 0x2A /* Z-axis magnetometer, hard-iron factor */
-#define ADIS16400_XMAGN_SIF 0x2C /* X-axis magnetometer, soft-iron factor */
-#define ADIS16400_YMAGN_SIF 0x2E /* Y-axis magnetometer, soft-iron factor */
-#define ADIS16400_ZMAGN_SIF 0x30 /* Z-axis magnetometer, soft-iron factor */
-
-#define ADIS16400_GPIO_CTRL 0x32 /* Auxiliary digital input/output control */
-#define ADIS16400_MSC_CTRL  0x34 /* Miscellaneous control */
-#define ADIS16400_SMPL_PRD  0x36 /* Internal sample period (rate) control */
-#define ADIS16400_SENS_AVG  0x38 /* Dynamic range and digital filter control */
-#define ADIS16400_SLP_CNT   0x3A /* Sleep mode control */
-#define ADIS16400_DIAG_STAT 0x3C /* System status */
+#define ADIS16400_XGYRO_OFF (0x1A >> 1) /* X-axis gyroscope bias offset factor */
+#define ADIS16400_YGYRO_OFF (0x1C >> 1) /* Y-axis gyroscope bias offset factor */
+#define ADIS16400_ZGYRO_OFF (0x1E >> 1) /* Z-axis gyroscope bias offset factor */
+#define ADIS16400_XACCL_OFF (0x20 >> 1) /* X-axis acceleration bias offset factor */
+#define ADIS16400_YACCL_OFF (0x22 >> 1) /* Y-axis acceleration bias offset factor */
+#define ADIS16400_ZACCL_OFF (0x24 >> 1) /* Z-axis acceleration bias offset factor */
+#define ADIS16400_XMAGN_HIF (0x26 >> 1) /* X-axis magnetometer, hard-iron factor */
+#define ADIS16400_YMAGN_HIF (0x28 >> 1) /* Y-axis magnetometer, hard-iron factor */
+#define ADIS16400_ZMAGN_HIF (0x2A >> 1) /* Z-axis magnetometer, hard-iron factor */
+#define ADIS16400_XMAGN_SIF (0x2C >> 1) /* X-axis magnetometer, soft-iron factor */
+#define ADIS16400_YMAGN_SIF (0x2E >> 1) /* Y-axis magnetometer, soft-iron factor */
+#define ADIS16400_ZMAGN_SIF (0x30 >> 1) /* Z-axis magnetometer, soft-iron factor */
+
+#define ADIS16400_GPIO_CTRL (0x32 >> 1) /* Auxiliary digital input/output control */
+#define ADIS16400_MSC_CTRL  (0x34 >> 1) /* Miscellaneous control */
+#define ADIS16400_SMPL_PRD  (0x36 >> 1) /* Internal sample period (rate) control */
+#define ADIS16400_SENS_AVG  (0x38 >> 1) /* Dynamic range and digital filter control */
+#define ADIS16400_SLP_CNT   (0x3A >> 1) /* Sleep mode control */
+#define ADIS16400_DIAG_STAT (0x3C >> 1) /* System status */
 
 /* Alarm functions */
-#define ADIS16400_GLOB_CMD  0x3E /* System command */
-#define ADIS16400_ALM_MAG1  0x40 /* Alarm 1 amplitude threshold */
-#define ADIS16400_ALM_MAG2  0x42 /* Alarm 2 amplitude threshold */
-#define ADIS16400_ALM_SMPL1 0x44 /* Alarm 1 sample size */
-#define ADIS16400_ALM_SMPL2 0x46 /* Alarm 2 sample size */
-#define ADIS16400_ALM_CTRL  0x48 /* Alarm control */
-#define ADIS16400_AUX_DAC   0x4A /* Auxiliary DAC data */
-
-#define ADIS16400_PRODUCT_ID 0x56 /* Product identifier */
+#define ADIS16400_GLOB_CMD  (0x3E >> 1) /* System command */
+#define ADIS16400_ALM_MAG1  (0x40 >> 1) /* Alarm 1 amplitude threshold */
+#define ADIS16400_ALM_MAG2  (0x42 >> 1) /* Alarm 2 amplitude threshold */
+#define ADIS16400_ALM_SMPL1 (0x44 >> 1) /* Alarm 1 sample size */
+#define ADIS16400_ALM_SMPL2 (0x46 >> 1) /* Alarm 2 sample size */
+#define ADIS16400_ALM_CTRL  (0x48 >> 1) /* Alarm control */
+#define ADIS16400_AUX_DAC   (0x4A >> 1) /* Auxiliary DAC data */
+
+#define ADIS16400_PRODUCT_ID (0x56 >> 1) /* Product identifier */
 
 #define ADIS16400_ERROR_ACTIVE			(1<<14)
 #define ADIS16400_NEW_DATA			(1<<14)
diff --git a/drivers/staging/iio/imu/adis16400_core.c b/drivers/staging/iio/imu/adis16400_core.c
index b4be380..a75a7d1 100644
--- a/drivers/staging/iio/imu/adis16400_core.c
+++ b/drivers/staging/iio/imu/adis16400_core.c
@@ -1074,7 +1074,7 @@ static struct regmap_config adis16400_regmap_config = {
 	.precious_reg = &adis16400_reg_precious,
 	.volatile_reg = &adis16400_reg_volatile,
 	.max_register = 0x56,
-	.write_flag_mask = 0x80,
+	.write_flag_mask = 0x40, /* doubling of this occurs */
 	.read_flag_mask = 0,
 };
 
diff --git a/drivers/staging/iio/imu/adis16400_ring.c b/drivers/staging/iio/imu/adis16400_ring.c
index 8345d4e..e1cd1bf 100644
--- a/drivers/staging/iio/imu/adis16400_ring.c
+++ b/drivers/staging/iio/imu/adis16400_ring.c
@@ -57,17 +57,17 @@ static int adis16400_spi_read_burst(struct device *dev, u8 *rx)
 }
 
 static const u16 read_all_tx_array[] = {
-	cpu_to_be16(ADIS16400_READ_REG(ADIS16400_SUPPLY_OUT)),
-	cpu_to_be16(ADIS16400_READ_REG(ADIS16400_XGYRO_OUT)),
-	cpu_to_be16(ADIS16400_READ_REG(ADIS16400_YGYRO_OUT)),
-	cpu_to_be16(ADIS16400_READ_REG(ADIS16400_ZGYRO_OUT)),
-	cpu_to_be16(ADIS16400_READ_REG(ADIS16400_XACCL_OUT)),
-	cpu_to_be16(ADIS16400_READ_REG(ADIS16400_YACCL_OUT)),
-	cpu_to_be16(ADIS16400_READ_REG(ADIS16400_ZACCL_OUT)),
-	cpu_to_be16(ADIS16400_READ_REG(ADIS16350_XTEMP_OUT)),
-	cpu_to_be16(ADIS16400_READ_REG(ADIS16350_YTEMP_OUT)),
-	cpu_to_be16(ADIS16400_READ_REG(ADIS16350_ZTEMP_OUT)),
-	cpu_to_be16(ADIS16400_READ_REG(ADIS16400_AUX_ADC)),
+	cpu_to_be16(ADIS16400_READ_REG(ADIS16400_SUPPLY_OUT << 1)),
+	cpu_to_be16(ADIS16400_READ_REG(ADIS16400_XGYRO_OUT << 1)),
+	cpu_to_be16(ADIS16400_READ_REG(ADIS16400_YGYRO_OUT << 1)),
+	cpu_to_be16(ADIS16400_READ_REG(ADIS16400_ZGYRO_OUT << 1)),
+	cpu_to_be16(ADIS16400_READ_REG(ADIS16400_XACCL_OUT << 1)),
+	cpu_to_be16(ADIS16400_READ_REG(ADIS16400_YACCL_OUT << 1)),
+	cpu_to_be16(ADIS16400_READ_REG(ADIS16400_ZACCL_OUT << 1)),
+	cpu_to_be16(ADIS16400_READ_REG(ADIS16350_XTEMP_OUT << 1)),
+	cpu_to_be16(ADIS16400_READ_REG(ADIS16350_YTEMP_OUT << 1)),
+	cpu_to_be16(ADIS16400_READ_REG(ADIS16350_ZTEMP_OUT << 1)),
+	cpu_to_be16(ADIS16400_READ_REG(ADIS16400_AUX_ADC << 1)),
 };
 
 static int adis16350_spi_read_all(struct device *dev, u8 *rx)
-- 
1.7.3.4

[PATCH 5/6] regmap-spi-adi generalize regmap_spi_read.

[Jonathan Cameron]

Complexity here is that we have to break the transers after every word
so that the chip select can go high.

Note this is NOT a burst read, but rather a interleaved
register read - there may well be better ways of doing this.

The burst read supported by for example the adis16400 is too odd
to fit well into regmap.
---
 drivers/base/regmap/regmap-spi-adi.c |   31 ++++++++++++++++++++++++++-----
 1 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/base/regmap/regmap-spi-adi.c b/drivers/base/regmap/regmap-spi-adi.c
index d4301cc..6b581fa 100644
--- a/drivers/base/regmap/regmap-spi-adi.c
+++ b/drivers/base/regmap/regmap-spi-adi.c
@@ -23,7 +23,7 @@ static int regmap_spi_write(struct device *dev, const void *data, size_t count)
 	int ret;
 	u8 *rawdata = (u8 *)data;
 
-	BUG_ON(count != 3);
+//	BUG_ON(count != 3);
 	/* Fiddling needed as the value is bigendian */
 	rawdata[0] = (rawdata[0] << 1) + 1;
 	ret = spi_write(spi, data, 2);
@@ -39,13 +39,34 @@ static int regmap_spi_read(struct device *dev,
 			   void *val, size_t val_size)
 {
 	struct spi_device *spi = to_spi_device(dev);
-	u8 *regraw = (u8 *)reg;
+	struct spi_transfer *xfers;
+	struct spi_message msg;
+	int i, ret;
+	u8 startreg = ((u8 *)reg)[0] << 1;
 
-	regraw[0] <<= 1;
+	/* Could keep a single register read special version */
+	xfers = kzalloc(sizeof(*xfers)*((val_size >> 1) + 1), GFP_KERNEL);
+	if (xfers == NULL)
+		return -ENOMEM;
+	
+	for (i = 0; i < val_size >> 1; i++) {
+		xfers[i].tx_buf = (u16 *)val + i;
+		((u8 *)val)[i*2] = startreg + 2*i;
+		((u8 *)val)[i*2 + 1] = 0;
+		xfers[i + 1].rx_buf = (u16 *)val + i;
+		xfers[i].bits_per_word = 8;
+		xfers[i].len = 2;
+	}
+	xfers[i].bits_per_word = 8;
+	xfers[i].len = 2;
+	spi_message_init(&msg);
+	for (i = 0; i < (val_size >> 1) + 1; i++)
+		spi_message_add_tail(&xfers[i], &msg);
 
-	BUG_ON(reg_size != 1 || val_size != 2);
+	ret = spi_sync(spi, &msg);
+	kfree(xfers);
 
-	return spi_write_then_read(spi, reg, 2, val, val_size);
+	return ret;
 }
 
 static struct regmap_bus regmap_spi_adi = {
-- 
1.7.3.4

[PATCH 6/6] staging:iio:imu:adis16400 make use of regmap bulk read capabilities

[Jonathan Cameron]

Note this only works because of the spi cs change default fun.
---
 drivers/staging/iio/imu/adis16400.h      |    4 +-
 drivers/staging/iio/imu/adis16400_ring.c |   67 +++++-------------------------
 2 files changed, 13 insertions(+), 58 deletions(-)

diff --git a/drivers/staging/iio/imu/adis16400.h b/drivers/staging/iio/imu/adis16400.h
index 919d6fa..d1b7b3e 100644
--- a/drivers/staging/iio/imu/adis16400.h
+++ b/drivers/staging/iio/imu/adis16400.h
@@ -122,7 +122,7 @@
 /* SLP_CNT */
 #define ADIS16400_SLP_CNT_POWER_OFF	(1<<8)
 
-#define ADIS16400_MAX_TX 24
+#define ADIS16400_MAX_TX 2
 #define ADIS16400_MAX_RX 24
 
 #define ADIS16400_SPI_SLOW	(u32)(300 * 1000)
@@ -158,7 +158,7 @@ struct adis16400_state {
 	struct adis16400_chip_info	*variant;
 
 	u8	tx[ADIS16400_MAX_TX] ____cacheline_aligned;
-	u8	rx[ADIS16400_MAX_RX] ____cacheline_aligned;
+	u16	rx[ADIS16400_MAX_RX >> 1] ____cacheline_aligned;
 };
 
 int adis16400_set_irq(struct iio_dev *indio_dev, bool enable);
diff --git a/drivers/staging/iio/imu/adis16400_ring.c b/drivers/staging/iio/imu/adis16400_ring.c
index e1cd1bf..84c539b 100644
--- a/drivers/staging/iio/imu/adis16400_ring.c
+++ b/drivers/staging/iio/imu/adis16400_ring.c
@@ -4,6 +4,7 @@
 #include <linux/spi/spi.h>
 #include <linux/slab.h>
 #include <linux/bitops.h>
+#include <linux/regmap.h>
 
 #include "../iio.h"
 #include "../ring_sw.h"
@@ -15,7 +16,7 @@
  * @dev: device associated with child of actual device (iio_dev or iio_trig)
  * @rx: somewhere to pass back the value read (min size is 24 bytes)
  **/
-static int adis16400_spi_read_burst(struct device *dev, u8 *rx)
+static int adis16400_spi_read_burst(struct device *dev, u16 *rx)
 {
 	struct spi_message msg;
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
@@ -56,55 +57,6 @@ static int adis16400_spi_read_burst(struct device *dev, u8 *rx)
 	return ret;
 }
 
-static const u16 read_all_tx_array[] = {
-	cpu_to_be16(ADIS16400_READ_REG(ADIS16400_SUPPLY_OUT << 1)),
-	cpu_to_be16(ADIS16400_READ_REG(ADIS16400_XGYRO_OUT << 1)),
-	cpu_to_be16(ADIS16400_READ_REG(ADIS16400_YGYRO_OUT << 1)),
-	cpu_to_be16(ADIS16400_READ_REG(ADIS16400_ZGYRO_OUT << 1)),
-	cpu_to_be16(ADIS16400_READ_REG(ADIS16400_XACCL_OUT << 1)),
-	cpu_to_be16(ADIS16400_READ_REG(ADIS16400_YACCL_OUT << 1)),
-	cpu_to_be16(ADIS16400_READ_REG(ADIS16400_ZACCL_OUT << 1)),
-	cpu_to_be16(ADIS16400_READ_REG(ADIS16350_XTEMP_OUT << 1)),
-	cpu_to_be16(ADIS16400_READ_REG(ADIS16350_YTEMP_OUT << 1)),
-	cpu_to_be16(ADIS16400_READ_REG(ADIS16350_ZTEMP_OUT << 1)),
-	cpu_to_be16(ADIS16400_READ_REG(ADIS16400_AUX_ADC << 1)),
-};
-
-static int adis16350_spi_read_all(struct device *dev, u8 *rx)
-{
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
-	struct adis16400_state *st = iio_priv(indio_dev);
-
-	struct spi_message msg;
-	int i, j = 0, ret;
-	struct spi_transfer *xfers;
-
-	xfers = kzalloc(sizeof(*xfers)*indio_dev->ring->scan_count + 1,
-			GFP_KERNEL);
-	if (xfers == NULL)
-		return -ENOMEM;
-
-	for (i = 0; i < ARRAY_SIZE(read_all_tx_array); i++)
-		if (test_bit(i, indio_dev->ring->scan_mask)) {
-			xfers[j].tx_buf = &read_all_tx_array[i];
-			xfers[j].bits_per_word = 16;
-			xfers[j].len = 2;
-			xfers[j + 1].rx_buf = rx + j*2;
-			j++;
-		}
-	xfers[j].bits_per_word = 16;
-	xfers[j].len = 2;
-
-	spi_message_init(&msg);
-	for (j = 0; j < indio_dev->ring->scan_count + 1; j++)
-		spi_message_add_tail(&xfers[j], &msg);
-
-	ret = spi_sync(st->us, &msg);
-	kfree(xfers);
-
-	return ret;
-}
-
 /* Whilst this makes a lot of calls to iio_sw_ring functions - it is to device
  * specific to be rolled into the core.
  */
@@ -120,7 +72,7 @@ static irqreturn_t adis16400_trigger_handler(int irq, void *p)
 	/* Asumption that long is enough for maximum channels */
 	unsigned long mask = *ring->scan_mask;
 
-	data = kmalloc(datasize , GFP_KERNEL);
+	data = kmalloc(datasize, GFP_KERNEL);
 	if (data == NULL) {
 		dev_err(&st->us->dev, "memory alloc failed in ring bh");
 		return -ENOMEM;
@@ -128,11 +80,14 @@ static irqreturn_t adis16400_trigger_handler(int irq, void *p)
 
 	if (ring->scan_count) {
 		if (st->variant->flags & ADIS16400_NO_BURST) {
-			ret = adis16350_spi_read_all(&indio_dev->dev, st->rx);
+			ret = regmap_bulk_read(st->regmap, ADIS16400_SUPPLY_OUT, st->rx, 12);
 			if (ret < 0)
 				goto err;
-			for (; i < ring->scan_count; i++)
-				data[i]	= *(s16 *)(st->rx + i*2);
+			for (; i < indio_dev->ring->scan_count; i++) {
+				j = __ffs(mask);
+				mask &= ~(1 << j);
+				data[i] = st->rx[j];
+			}
 		} else {
 			ret = adis16400_spi_read_burst(&indio_dev->dev, st->rx);
 			if (ret < 0)
@@ -141,9 +96,10 @@ static irqreturn_t adis16400_trigger_handler(int irq, void *p)
 				j = __ffs(mask);
 				mask &= ~(1 << j);
 				data[i] = be16_to_cpup(
-					(__be16 *)&(st->rx[j*2]));
+					(__be16 *)&(st->rx[j]));
 			}
 		}
+
 	}
 	/* Guaranteed to be aligned with 8 byte boundary */
 	if (ring->scan_timestamp)
@@ -175,7 +131,6 @@ static const struct iio_ring_setup_ops adis16400_ring_setup_ops = {
 int adis16400_configure_ring(struct iio_dev *indio_dev)
 {
 	int ret = 0;
-	struct adis16400_state *st = iio_priv(indio_dev);
 	struct iio_ring_buffer *ring;
 
 	ring = iio_sw_rb_allocate(indio_dev);
-- 
1.7.3.4

Re: [PATCH 3/6] staging:iio:imu: adis16400 partial conversion to regmap.

[Jonathan Cameron]

On 09/07/11 17:19, Jonathan Cameron wrote:
> This requires the regmap-spi-adi regmap bus to deal with
> complex read / write combinations.

Forgot to mention that I cheated here in that all supported
devices claim all registers exist though for some they
are undefined.   The devices won't actually try reading from
them and it won't do any harm if the debug code does so.  Just
won't get meaningful values for non existent sensors output
registers.  Can clean this up, but at the cost of a fair bit
of complexity as will need to work it out from the chan_spec
array.
> 
> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
> ---
>  drivers/staging/iio/imu/Kconfig          |    1 +
>  drivers/staging/iio/imu/adis16400.h      |    2 +
>  drivers/staging/iio/imu/adis16400_core.c |  300 ++++++++++++++++++-----------
>  drivers/staging/iio/imu/adis16400_ring.c |    2 +-
>  4 files changed, 190 insertions(+), 115 deletions(-)
> 
> diff --git a/drivers/staging/iio/imu/Kconfig b/drivers/staging/iio/imu/Kconfig
> index e0e0144..7dfaa43 100644
> --- a/drivers/staging/iio/imu/Kconfig
> +++ b/drivers/staging/iio/imu/Kconfig
> @@ -6,6 +6,7 @@ comment "Inertial measurement units"
>  config ADIS16400
>  	tristate "Analog Devices ADIS16400 and similar IMU SPI driver"
>  	depends on SPI
> +	select REGMAP_SPI_ADI
>  	select IIO_SW_RING if IIO_RING_BUFFER
>  	select IIO_TRIGGER if IIO_RING_BUFFER
>  	help
> diff --git a/drivers/staging/iio/imu/adis16400.h b/drivers/staging/iio/imu/adis16400.h
> index 1f8f0c6..0bf1fd9 100644
> --- a/drivers/staging/iio/imu/adis16400.h
> +++ b/drivers/staging/iio/imu/adis16400.h
> @@ -141,6 +141,7 @@ struct adis16400_chip_info {
>  	unsigned long default_scan_mask;
>  };
>  
> +struct regmap;
>  /**
>   * struct adis16400_state - device instance specific data
>   * @us:			actual spi_device
> @@ -150,6 +151,7 @@ struct adis16400_chip_info {
>   * @buf_lock:		mutex to protect tx and rx
>   **/
>  struct adis16400_state {
> +	struct regmap *regmap;
>  	struct spi_device		*us;
>  	struct iio_trigger		*trig;
>  	struct mutex			buf_lock;
> diff --git a/drivers/staging/iio/imu/adis16400_core.c b/drivers/staging/iio/imu/adis16400_core.c
> index b6d824f..b4be380 100644
> --- a/drivers/staging/iio/imu/adis16400_core.c
> +++ b/drivers/staging/iio/imu/adis16400_core.c
> @@ -25,6 +25,8 @@
>  #include <linux/sysfs.h>
>  #include <linux/list.h>
>  #include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/err.h>
>  
>  #include "../iio.h"
>  #include "../sysfs.h"
> @@ -42,28 +44,6 @@ enum adis16400_chip_variant {
>  	ADIS16400,
>  };
>  
> -/**
> - * adis16400_spi_write_reg_8() - write single byte to a register
> - * @dev: device associated with child of actual device (iio_dev or iio_trig)
> - * @reg_address: the address of the register to be written
> - * @val: the value to write
> - */
> -static int adis16400_spi_write_reg_8(struct iio_dev *indio_dev,
> -				     u8 reg_address,
> -				     u8 val)
> -{
> -	int ret;
> -	struct adis16400_state *st = iio_priv(indio_dev);
> -
> -	mutex_lock(&st->buf_lock);
> -	st->tx[0] = ADIS16400_WRITE_REG(reg_address);
> -	st->tx[1] = val;
> -
> -	ret = spi_write(st->us, st->tx, 2);
> -	mutex_unlock(&st->buf_lock);
> -
> -	return ret;
> -}
>  
>  /**
>   * adis16400_spi_write_reg_16() - write 2 bytes to a pair of registers
> @@ -71,43 +51,14 @@ static int adis16400_spi_write_reg_8(struct iio_dev *indio_dev,
>   * @reg_address: the address of the lower of the two registers. Second register
>   *               is assumed to have address one greater.
>   * @val: value to be written
> - *
> - * At the moment the spi framework doesn't allow global setting of cs_change.
> - * This means that use cannot be made of spi_write.
>   */
>  static int adis16400_spi_write_reg_16(struct iio_dev *indio_dev,
> -		u8 lower_reg_address,
> -		u16 value)
> +				      u8 lower_reg_address,
> +				      u16 value)
>  {
> -	int ret;
> -	struct spi_message msg;
>  	struct adis16400_state *st = iio_priv(indio_dev);
> -	struct spi_transfer xfers[] = {
> -		{
> -			.tx_buf = st->tx,
> -			.bits_per_word = 8,
> -			.len = 2,
> -			.cs_change = 1,
> -		}, {
> -			.tx_buf = st->tx + 2,
> -			.bits_per_word = 8,
> -			.len = 2,
> -		},
> -	};
> -
> -	mutex_lock(&st->buf_lock);
> -	st->tx[0] = ADIS16400_WRITE_REG(lower_reg_address);
> -	st->tx[1] = value & 0xFF;
> -	st->tx[2] = ADIS16400_WRITE_REG(lower_reg_address + 1);
> -	st->tx[3] = (value >> 8) & 0xFF;
>  
> -	spi_message_init(&msg);
> -	spi_message_add_tail(&xfers[0], &msg);
> -	spi_message_add_tail(&xfers[1], &msg);
> -	ret = spi_sync(st->us, &msg);
> -	mutex_unlock(&st->buf_lock);
> -
> -	return ret;
> +	return regmap_write(st->regmap, lower_reg_address, value);
>  }
>  
>  /**
> @@ -116,49 +67,22 @@ static int adis16400_spi_write_reg_16(struct iio_dev *indio_dev,
>   * @reg_address: the address of the lower of the two registers. Second register
>   *               is assumed to have address one greater.
>   * @val: somewhere to pass back the value read
> - *
> - * At the moment the spi framework doesn't allow global setting of cs_change.
> - * This means that use cannot be made of spi_read.
>   **/
>  static int adis16400_spi_read_reg_16(struct iio_dev *indio_dev,
> -		u8 lower_reg_address,
> -		u16 *val)
> +				     u8 lower_reg_address,
> +				     u16 *val)
>  {
> -	struct spi_message msg;
>  	struct adis16400_state *st = iio_priv(indio_dev);
>  	int ret;
> -	struct spi_transfer xfers[] = {
> -		{
> -			.tx_buf = st->tx,
> -			.bits_per_word = 8,
> -			.len = 2,
> -			.cs_change = 1,
> -		}, {
> -			.rx_buf = st->rx,
> -			.bits_per_word = 8,
> -			.len = 2,
> -		},
> -	};
> +	unsigned int value;
>  
> -	mutex_lock(&st->buf_lock);
> -	st->tx[0] = ADIS16400_READ_REG(lower_reg_address);
> -	st->tx[1] = 0;
> -
> -	spi_message_init(&msg);
> -	spi_message_add_tail(&xfers[0], &msg);
> -	spi_message_add_tail(&xfers[1], &msg);
> -	ret = spi_sync(st->us, &msg);
> -	if (ret) {
> -		dev_err(&st->us->dev,
> -			"problem when reading 16 bit register 0x%02X",
> -			lower_reg_address);
> -		goto error_ret;
> -	}
> -	*val = (st->rx[0] << 8) | st->rx[1];
> +	ret = regmap_read(st->regmap, lower_reg_address, &value);
> +	if (ret < 0)
> +		return ret;
>  
> -error_ret:
> -	mutex_unlock(&st->buf_lock);
> -	return ret;
> +	*val = value;
> +
> +	return 0;
>  }
>  
>  static ssize_t adis16400_read_frequency(struct device *dev,
> @@ -172,7 +96,7 @@ static ssize_t adis16400_read_frequency(struct device *dev,
>  	ret = adis16400_spi_read_reg_16(indio_dev,
>  			ADIS16400_SMPL_PRD,
>  			&t);
> -	if (ret)
> +	if (ret < 0)
>  		return ret;
>  	sps =  (t & ADIS16400_SMPL_PRD_TIME_BASE) ? 53 : 1638;
>  	sps /= (t & ADIS16400_SMPL_PRD_DIV_MASK) + 1;
> @@ -192,7 +116,7 @@ static ssize_t adis16400_write_frequency(struct device *dev,
>  	u8 t;
>  
>  	ret = strict_strtol(buf, 10, &val);
> -	if (ret)
> +	if (ret < 0)
>  		return ret;
>  
>  	mutex_lock(&indio_dev->mlock);
> @@ -206,22 +130,22 @@ static ssize_t adis16400_write_frequency(struct device *dev,
>  	else
>  		st->us->max_speed_hz = ADIS16400_SPI_FAST;
>  
> -	ret = adis16400_spi_write_reg_8(indio_dev,
> +	ret = adis16400_spi_write_reg_16(indio_dev,
>  			ADIS16400_SMPL_PRD,
>  			t);
>  
>  	mutex_unlock(&indio_dev->mlock);
>  
> -	return ret ? ret : len;
> +	return ret < 0 ? ret : len;
>  }
>  
>  static int adis16400_reset(struct iio_dev *indio_dev)
>  {
>  	int ret;
> -	ret = adis16400_spi_write_reg_8(indio_dev,
> +	ret = adis16400_spi_write_reg_16(indio_dev,
>  			ADIS16400_GLOB_CMD,
>  			ADIS16400_GLOB_CMD_SW_RESET);
> -	if (ret)
> +	if (ret < 0)
>  		dev_err(&indio_dev->dev, "problem resetting device");
>  
>  	return ret;
> @@ -252,7 +176,7 @@ int adis16400_set_irq(struct iio_dev *indio_dev, bool enable)
>  	u16 msc;
>  
>  	ret = adis16400_spi_read_reg_16(indio_dev, ADIS16400_MSC_CTRL, &msc);
> -	if (ret)
> +	if (ret < 0)
>  		goto error_ret;
>  
>  	msc |= ADIS16400_MSC_CTRL_DATA_RDY_POL_HIGH;
> @@ -262,8 +186,6 @@ int adis16400_set_irq(struct iio_dev *indio_dev, bool enable)
>  		msc &= ~ADIS16400_MSC_CTRL_DATA_RDY_EN;
>  
>  	ret = adis16400_spi_write_reg_16(indio_dev, ADIS16400_MSC_CTRL, msc);
> -	if (ret)
> -		goto error_ret;
>  
>  error_ret:
>  	return ret;
> @@ -338,7 +260,7 @@ static int adis16400_self_test(struct iio_dev *indio_dev)
>  	ret = adis16400_spi_write_reg_16(indio_dev,
>  			ADIS16400_MSC_CTRL,
>  			ADIS16400_MSC_CTRL_MEM_TEST);
> -	if (ret) {
> +	if (ret < 0) {
>  		dev_err(&indio_dev->dev, "problem starting self test");
>  		goto err_ret;
>  	}
> @@ -362,24 +284,24 @@ static int adis16400_initial_setup(struct iio_dev *indio_dev)
>  	spi_setup(st->us);
>  
>  	ret = adis16400_set_irq(indio_dev, false);
> -	if (ret) {
> +	if (ret < 0) {
>  		dev_err(&indio_dev->dev, "disable irq failed");
>  		goto err_ret;
>  	}
>  
>  	ret = adis16400_self_test(indio_dev);
> -	if (ret) {
> +	if (ret < 0) {
>  		dev_err(&indio_dev->dev, "self test failure");
>  		goto err_ret;
>  	}
>  
>  	ret = adis16400_check_status(indio_dev);
> -	if (ret) {
> +	if (ret < 0) {
>  		adis16400_reset(indio_dev);
>  		dev_err(&indio_dev->dev, "device not playing ball -> reset");
>  		msleep(ADIS16400_STARTUP_DELAY);
>  		ret = adis16400_check_status(indio_dev);
> -		if (ret) {
> +		if (ret < 0) {
>  			dev_err(&indio_dev->dev, "giving up");
>  			goto err_ret;
>  		}
> @@ -387,7 +309,7 @@ static int adis16400_initial_setup(struct iio_dev *indio_dev)
>  	if (st->variant->flags & ADIS16400_HAS_PROD_ID) {
>  		ret = adis16400_spi_read_reg_16(indio_dev,
>  						ADIS16400_PRODUCT_ID, &prod_id);
> -		if (ret)
> +		if (ret < 0)
>  			goto err_ret;
>  
>  		if ((prod_id & 0xF000) != st->variant->product_id)
> @@ -492,7 +414,7 @@ static int adis16400_read_raw(struct iio_dev *indio_dev,
>  		ret = adis16400_spi_read_reg_16(indio_dev,
>  				adis16400_addresses[chan->address][0],
>  				&val16);
> -		if (ret) {
> +		if (ret < 0) {
>  			mutex_unlock(&indio_dev->mlock);
>  			return ret;
>  		}
> @@ -539,7 +461,7 @@ static int adis16400_read_raw(struct iio_dev *indio_dev,
>  				adis16400_addresses[chan->address][1],
>  				&val16);
>  		mutex_unlock(&indio_dev->mlock);
> -		if (ret)
> +		if (ret < 0)
>  			return ret;
>  		val16 = ((val16 & 0xFFF) << 4) >> 4;
>  		*val = val16;
> @@ -1016,6 +938,146 @@ static const struct iio_info adis16400_info = {
>  	.attrs = &adis16400_attribute_group,
>  };
>  
> +/* This may technically result in read attempts to undefined registers */
> +static bool adis16400_reg_readable(struct device *dev,
> +				   unsigned int reg)
> +{
> +	switch (reg) {
> +	case ADIS16400_FLASH_CNT:
> +	case ADIS16400_SUPPLY_OUT:
> +	case ADIS16400_XGYRO_OUT:
> +	case ADIS16400_YGYRO_OUT:
> +	case ADIS16400_ZGYRO_OUT:
> +	case ADIS16400_XACCL_OUT:
> +	case ADIS16400_YACCL_OUT:
> +	case ADIS16400_ZACCL_OUT:
> +	case ADIS16400_XMAGN_OUT:
> +	case ADIS16400_YMAGN_OUT:
> +	case ADIS16400_ZMAGN_OUT:
> +	case ADIS16400_TEMP_OUT:
> +	case ADIS16400_AUX_ADC:
> +	case ADIS16400_XGYRO_OFF:
> +	case ADIS16400_YGYRO_OFF:
> +	case ADIS16400_ZGYRO_OFF:
> +	case ADIS16400_XACCL_OFF:
> +	case ADIS16400_YACCL_OFF:
> +	case ADIS16400_ZACCL_OFF:
> +	case ADIS16400_XMAGN_HIF:
> +	case ADIS16400_YMAGN_HIF:
> +	case ADIS16400_ZMAGN_HIF:
> +	case ADIS16400_XMAGN_SIF:
> +	case ADIS16400_YMAGN_SIF:
> +	case ADIS16400_ZMAGN_SIF:
> +	case ADIS16400_GPIO_CTRL:
> +	case ADIS16400_MSC_CTRL:
> +	case ADIS16400_SMPL_PRD:
> +	case ADIS16400_SENS_AVG:
> +	case ADIS16400_DIAG_STAT:
> +	case ADIS16400_ALM_MAG1:
> +	case ADIS16400_ALM_MAG2:
> +	case ADIS16400_ALM_SMPL1:
> +	case ADIS16400_ALM_SMPL2:
> +	case ADIS16400_ALM_CTRL:
> +	case ADIS16400_AUX_DAC:
> +	case ADIS16400_PRODUCT_ID:
> +		return true;
> +	default:
> +		return false;
> +	};
> +}
> +
> +static bool adis16400_reg_precious(struct device *dev,
> +				   unsigned int reg)
> +{
> +	switch (reg) {
> +	case ADIS16400_SUPPLY_OUT:
> +	case ADIS16400_XGYRO_OUT:
> +	case ADIS16400_YGYRO_OUT:
> +	case ADIS16400_ZGYRO_OUT:
> +	case ADIS16400_XACCL_OUT:
> +	case ADIS16400_YACCL_OUT:
> +	case ADIS16400_ZACCL_OUT:
> +	case ADIS16400_XMAGN_OUT:
> +	case ADIS16400_YMAGN_OUT:
> +	case ADIS16400_ZMAGN_OUT:
> +	case ADIS16400_TEMP_OUT:
> +	case ADIS16400_AUX_ADC:
> +	case ADIS16400_DIAG_STAT:
> +		return true;
> +	default:
> +		return 0;
> +	}
> +}
> +
> +static bool adis16400_reg_volatile(struct device *dev,
> +				   unsigned int reg)
> +{
> +	switch (reg) {
> +	case ADIS16400_FLASH_CNT:
> +	case ADIS16400_SUPPLY_OUT:
> +	case ADIS16400_XGYRO_OUT:
> +	case ADIS16400_YGYRO_OUT:
> +	case ADIS16400_ZGYRO_OUT:
> +	case ADIS16400_XACCL_OUT:
> +	case ADIS16400_YACCL_OUT:
> +	case ADIS16400_ZACCL_OUT:
> +	case ADIS16400_XMAGN_OUT:
> +	case ADIS16400_YMAGN_OUT:
> +	case ADIS16400_ZMAGN_OUT:
> +	case ADIS16400_TEMP_OUT:
> +	case ADIS16400_AUX_ADC:
> +	case ADIS16400_DIAG_STAT:
> +
> +		return true;
> +	default:
> +		return 0;
> +	}
> +}
> +static bool adis16400_reg_writeable(struct device *dev,
> +				    unsigned int reg)
> +{
> +	switch (reg) {
> +	case ADIS16400_XGYRO_OFF:
> +	case ADIS16400_YGYRO_OFF:
> +	case ADIS16400_ZGYRO_OFF:
> +	case ADIS16400_XACCL_OFF:
> +	case ADIS16400_YACCL_OFF:
> +	case ADIS16400_ZACCL_OFF:
> +	case ADIS16400_XMAGN_HIF:
> +	case ADIS16400_YMAGN_HIF:
> +	case ADIS16400_ZMAGN_HIF:
> +	case ADIS16400_XMAGN_SIF:
> +	case ADIS16400_YMAGN_SIF:
> +	case ADIS16400_ZMAGN_SIF:
> +	case ADIS16400_GPIO_CTRL:
> +	case ADIS16400_MSC_CTRL:
> +	case ADIS16400_SMPL_PRD:
> +	case ADIS16400_SENS_AVG:
> +	case ADIS16400_SLP_CNT:
> +	case ADIS16400_GLOB_CMD:
> +	case ADIS16400_ALM_MAG1:
> +	case ADIS16400_ALM_MAG2:
> +	case ADIS16400_ALM_SMPL1:
> +	case ADIS16400_ALM_SMPL2:
> +	case ADIS16400_ALM_CTRL:
> +	case ADIS16400_AUX_DAC:
> +		return true;
> +	default:
> +		return false;
> +	};
> +}
> +static struct regmap_config adis16400_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 16,
> +	.writeable_reg = &adis16400_reg_writeable,
> +	.readable_reg = &adis16400_reg_readable,
> +	.precious_reg = &adis16400_reg_precious,
> +	.volatile_reg = &adis16400_reg_volatile,
> +	.max_register = 0x56,
> +	.write_flag_mask = 0x80,
> +	.read_flag_mask = 0,
> +};
> +
>  static int __devinit adis16400_probe(struct spi_device *spi)
>  {
>  	int ret;
> @@ -1026,8 +1088,14 @@ static int __devinit adis16400_probe(struct spi_device *spi)
>  		goto error_ret;
>  	}
>  	st = iio_priv(indio_dev);
> +	st->regmap = regmap_init_spi_adi(spi, &adis16400_regmap_config);
> +	if (IS_ERR(st->regmap)) {
> +		ret = PTR_ERR(st->regmap);
> +		goto error_free_dev;
> +	}
>  	/* this is only used for removal purposes */
>  	spi_set_drvdata(spi, indio_dev);
> +	spi->cs_between_transfers = 1;
>  
>  	st->us = spi;
>  	mutex_init(&st->buf_lock);
> @@ -1042,29 +1110,29 @@ static int __devinit adis16400_probe(struct spi_device *spi)
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  
>  	ret = adis16400_configure_ring(indio_dev);
> -	if (ret)
> -		goto error_free_dev;
> +	if (ret < 0)
> +		goto error_free_regmap;
>  
>  	ret = iio_ring_buffer_register(indio_dev,
>  				       st->variant->channels,
>  				       st->variant->num_channels);
> -	if (ret) {
> +	if (ret < 0) {
>  		dev_err(&spi->dev, "failed to initialize the ring\n");
>  		goto error_unreg_ring_funcs;
>  	}
>  
>  	if (spi->irq) {
>  		ret = adis16400_probe_trigger(indio_dev);
> -		if (ret)
> +		if (ret < 0)
>  			goto error_uninitialize_ring;
>  	}
>  
>  	/* Get the device into a sane initial state */
>  	ret = adis16400_initial_setup(indio_dev);
> -	if (ret)
> +	if (ret < 0)
>  		goto error_remove_trigger;
>  	ret = iio_device_register(indio_dev);
> -	if (ret)
> +	if (ret < 0)
>  		goto error_remove_trigger;
>  
>  	return 0;
> @@ -1076,6 +1144,8 @@ error_uninitialize_ring:
>  	iio_ring_buffer_unregister(indio_dev);
>  error_unreg_ring_funcs:
>  	adis16400_unconfigure_ring(indio_dev);
> +error_free_regmap:
> +	regmap_exit(st->regmap);
>  error_free_dev:
>  	iio_free_device(indio_dev);
>  error_ret:
> @@ -1087,14 +1157,16 @@ static int adis16400_remove(struct spi_device *spi)
>  {
>  	int ret;
>  	struct iio_dev *indio_dev =  spi_get_drvdata(spi);
> +	struct adis16400_state *st = iio_priv(indio_dev);
>  
>  	ret = adis16400_stop_device(indio_dev);
> -	if (ret)
> +	if (ret < 0)
>  		goto err_ret;
>  
>  	adis16400_remove_trigger(indio_dev);
>  	iio_ring_buffer_unregister(indio_dev);
>  	adis16400_unconfigure_ring(indio_dev);
> +	regmap_exit(st->regmap);
>  	iio_device_unregister(indio_dev);
>  
>  	return 0;
> diff --git a/drivers/staging/iio/imu/adis16400_ring.c b/drivers/staging/iio/imu/adis16400_ring.c
> index af25697..8345d4e 100644
> --- a/drivers/staging/iio/imu/adis16400_ring.c
> +++ b/drivers/staging/iio/imu/adis16400_ring.c
> @@ -47,7 +47,7 @@ static int adis16400_spi_read_burst(struct device *dev, u8 *rx)
>  	spi_setup(st->us);
>  
>  	ret = spi_sync(st->us, &msg);
> -	if (ret)
> +	if (ret < 0)
>  		dev_err(&st->us->dev, "problem when burst reading");
>  
>  	st->us->max_speed_hz = old_speed_hz;

Re: [PATCH 1/6] SPI: add ability to say we want a cs change after every transfer.

[Mark Brown]

On Wed, Sep 07, 2011 at 05:19:42PM +0100, Jonathan Cameron wrote:
> This allows a number of drivers to make use of utility functions
> such spi_write_then_read as well as making use of regmap possible.
> 
> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>

We could add this in regmap if required.

Re: [PATCH 2/6] regmap: Add a magic bus type to handle quirks of analog devices ADIS sensors.

[Mark Brown]

On Wed, Sep 07, 2011 at 05:19:43PM +0100, Jonathan Cameron wrote:
> These devices look like 8 bit registers for writes and 16 bit registers for
> reads.  As you might imagine this causes some 'issues' hence this regmap
> bus implementation claims they are always 16bit and does the mangling to
> make the writes work.
> ---

You've not signed this off.  To be honest I'm not terribly happy about
pushing this into the regmap core code; if we start needing to do stuff
like this we should expose the bus interface.

> +
> +static int regmap_spi_write(struct device *dev, const void *data, size_t count)
> +{
> +/* Now this only works for 8 bit addresss 16 bit register first byte of data
> + * is the lower address, second two the value */
> +	struct spi_device *spi = to_spi_device(dev);
> +	int ret;

Indentation.

> +static struct regmap_bus regmap_spi_adi = {
> +	.write = regmap_spi_write,
> +	.read = regmap_spi_read,
> +};

You want to implement the gather write too if you can.

Re: Blockers on IIO usage of regmap.

[Mark Brown]

On Wed, Sep 07, 2011 at 05:10:24PM +0100, Jonathan Cameron wrote:

Please fix your mailer to word wrap at less than 80 colums, they're
really quite hard to read as a result of this.

> CS -_______________________-
> TX  Ada0...Ada7  Da0....Da7
> RX XXXXXXXXXXXXXXXXXXXXXXXXX
> 
> Reads are 16 bit with either of the two 8 bit register addresses given the same value
> 
> CS -______________________-_____________________-
> TX  Ada0....Ada7 XXXXXXXX
> RX  XXXXXXXXXXXXXXXXXXXXX  Da0...Da7  Db0....Db7  

> Can interpret Da0...Da7 and Db0....Db7 as single 16 bit register and consider this device
> to just have a weird write method and normal read.  Might be easier.  I'll define Ax<n> as
> 16 bit address for the burst read.

This is starting to seem pretty far off the reservation.

> Perhaps the burst mode thing is better handled by just providing a hook to allow data to be pushed
> into regmap (from 'magic' sources), but the weird write read combination looks to me like something
> that makes sense to have in regmap (be it as another bus variant).

Probably not as a bus, it sounds like a marshalling difference rather
than a bus - the buses should really only understand byte streams.  I
don't have any bright ideas on how to deal with this, it's fairly far
away from the problem space I'm worried about.

Re: Blockers on IIO usage of regmap.

[Jonathan Cameron]

On 09/07/11 18:57, Mark Brown wrote:
> On Wed, Sep 07, 2011 at 05:10:24PM +0100, Jonathan Cameron wrote:
> 
> Please fix your mailer to word wrap at less than 80 colums, they're
> really quite hard to read as a result of this.
> 
>> CS -_______________________-
>> TX  Ada0...Ada7  Da0....Da7
>> RX XXXXXXXXXXXXXXXXXXXXXXXXX
>>
>> Reads are 16 bit with either of the two 8 bit register addresses given the same value
>>
>> CS -______________________-_____________________-
>> TX  Ada0....Ada7 XXXXXXXX
>> RX  XXXXXXXXXXXXXXXXXXXXX  Da0...Da7  Db0....Db7  
> 
>> Can interpret Da0...Da7 and Db0....Db7 as single 16 bit register and consider this device
>> to just have a weird write method and normal read.  Might be easier.  I'll define Ax<n> as
>> 16 bit address for the burst read.
> 
> This is starting to seem pretty far off the reservation.
Indeed.
> 
>> Perhaps the burst mode thing is better handled by just providing a hook to allow data to be pushed
>> into regmap (from 'magic' sources), but the weird write read combination looks to me like something
>> that makes sense to have in regmap (be it as another bus variant).
> 
> Probably not as a bus, it sounds like a marshalling difference rather
> than a bus - the buses should really only understand byte streams.  I
> don't have any bright ideas on how to deal with this, it's fairly far
> away from the problem space I'm worried about.
The right answer maybe that it isn't a good idea to do it at all, but it
wasn't obvious until I tried! Having done it I'm not sure either way.

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

Re: [PATCH 1/6] SPI: add ability to say we want a cs change after every transfer.

[Mark Brown]

On Wed, Sep 07, 2011 at 07:14:15PM +0100, Jonathan Cameron wrote:
> On 09/07/11 18:35, Mark Brown wrote:

> > We could add this in regmap if required.

> Makes more sense in spi and has been in their 'may be needed comment'
> from the first.  Also, lots of other users who don't fit into regmap
> (because they don't have any registers to speak of :)

Right, but obviously anything doing SPI transfers can just set the flag
in the transfers anyway.  It looked like you were adding this to the
core due to other intervening abstractions, probably this is just a
problem with your changelog.

Re: [PATCH 1/6] SPI: add ability to say we want a cs change after every transfer.

[Jonathan Cameron]

On 09/07/11 18:35, Mark Brown wrote:
> On Wed, Sep 07, 2011 at 05:19:42PM +0100, Jonathan Cameron wrote:
>> This allows a number of drivers to make use of utility functions
>> such spi_write_then_read as well as making use of regmap possible.
>>
>> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
> 
> We could add this in regmap if required.
Makes more sense in spi and has been in their 'may be needed comment'
from the first.  Also, lots of other users who don't fit into regmap
(because they don't have any registers to speak of :)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 2/6] regmap: Add a magic bus type to handle quirks of analog devices ADIS sensors.

[Jonathan Cameron]

On 09/07/11 18:47, Mark Brown wrote:
> On Wed, Sep 07, 2011 at 05:19:43PM +0100, Jonathan Cameron wrote:
>> These devices look like 8 bit registers for writes and 16 bit registers for
>> reads.  As you might imagine this causes some 'issues' hence this regmap
>> bus implementation claims they are always 16bit and does the mangling to
>> make the writes work.
>> ---
> 
> You've not signed this off.  
Indeed.  Hence I'm not happy with it :)  Just put it out there for comment.
Weird spi usage is hardly unusual so these issues were bound to pop up
at some point.

> To be honest I'm not terribly happy about
> pushing this into the regmap core code; if we start needing to do stuff
> like this we should expose the bus interface.
That's certainly an option, but I'd really like to use the regmap caching
stuff in here.  These things can have quite a few registers that other than
their weird read / write quirks look much like any other register based
device. (particularly ignoring the burst reads but they tend to apply
to volatile registers only so caching is probably irrelevant).

At the moment, the only hooks AFAIKS to allow this are at the bus level.
I'm not sure where else they could go. (I haven't actually looked much
at the cache code yet though).

Actually I may have misunderstood, do you mean expose the bus interface
within regmap or just not use regmap at all?

> 
>> +
>> +static int regmap_spi_write(struct device *dev, const void *data, size_t count)
>> +{
>> +/* Now this only works for 8 bit addresss 16 bit register first byte of data
>> + * is the lower address, second two the value */
>> +	struct spi_device *spi = to_spi_device(dev);
>> +	int ret;
> 
> Indentation.
> 
>> +static struct regmap_bus regmap_spi_adi = {
>> +	.write = regmap_spi_write,
>> +	.read = regmap_spi_read,
>> +};
> 
> You want to implement the gather write too if you can.
Doesn't really exist other than by linearising them into a series
of calls to the write function. (assuming I understand what those
functions are doing right!)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 1/6] SPI: add ability to say we want a cs change after every transfer.

[Jonathan Cameron]

On 09/07/11 19:12, Mark Brown wrote:
> On Wed, Sep 07, 2011 at 07:14:15PM +0100, Jonathan Cameron wrote:
>> On 09/07/11 18:35, Mark Brown wrote:
> 
>>> We could add this in regmap if required.
> 
>> Makes more sense in spi and has been in their 'may be needed comment'
>> from the first.  Also, lots of other users who don't fit into regmap
>> (because they don't have any registers to speak of :)
> 
> Right, but obviously anything doing SPI transfers can just set the flag
> in the transfers anyway.  It looked like you were adding this to the
> core due to other intervening abstractions, probably this is just a
> problem with your changelog.
I'll make it clearer that it has uses there elsewhere as well not to
mention merely stopping stupid bugs in drivers where this is always
needed.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 2/6] regmap: Add a magic bus type to handle quirks of analog devices ADIS sensors.

[Mark Brown]

On Wed, Sep 07, 2011 at 07:26:10PM +0100, Jonathan Cameron wrote:
> On 09/07/11 18:47, Mark Brown wrote:
> > On Wed, Sep 07, 2011 at 05:19:43PM +0100, Jonathan Cameron wrote:

> > To be honest I'm not terribly happy about
> > pushing this into the regmap core code; if we start needing to do stuff
> > like this we should expose the bus interface.

> That's certainly an option, but I'd really like to use the regmap caching
> stuff in here.  These things can have quite a few registers that other than
> their weird read / write quirks look much like any other register based
> device. (particularly ignoring the burst reads but they tend to apply
> to volatile registers only so caching is probably irrelevant).

> At the moment, the only hooks AFAIKS to allow this are at the bus level.
> I'm not sure where else they could go. (I haven't actually looked much
> at the cache code yet though).

Well, if there's not hooks they could be added if it's not too painful.
However I'm not convinced that's sane.

> Actually I may have misunderstood, do you mean expose the bus interface
> within regmap or just not use regmap at all?

Either.  Or either expose marshalling or add configuration for the
marshalling depending on what's being done, I'm not convinced you need
to be working at SPI level at all.

> >> +static struct regmap_bus regmap_spi_adi = {
> >> +	.write = regmap_spi_write,
> >> +	.read = regmap_spi_read,
> >> +};

> > You want to implement the gather write too if you can.

> Doesn't really exist other than by linearising them into a series
> of calls to the write function. (assuming I understand what those
> functions are doing right!)

If you build up a series of SPI transfers you just need to control when
/CS gets bounced appropriately.

Re: [PATCH 2/6] regmap: Add a magic bus type to handle quirks of analog devices ADIS sensors.

[Mark Brown]

On Wed, Sep 07, 2011 at 07:50:07PM +0100, Jonathan Cameron wrote:
> On 09/07/11 19:32, Mark Brown wrote:

> > Either.  Or either expose marshalling or add configuration for the
> > marshalling depending on what's being done, I'm not convinced you need
> > to be working at SPI level at all.

> I'm a little unclear what you mean by 'marshalling'.  Could you expand
> on how that might work?

Marshalling is the process of formatting data for transmit and parsing
it once it's been read in.

Re: [PATCH 2/6] regmap: Add a magic bus type to handle quirks of analog devices ADIS sensors.

[Jonathan Cameron]

On 09/07/11 19:32, Mark Brown wrote:
> On Wed, Sep 07, 2011 at 07:26:10PM +0100, Jonathan Cameron wrote:
>> On 09/07/11 18:47, Mark Brown wrote:
>>> On Wed, Sep 07, 2011 at 05:19:43PM +0100, Jonathan Cameron wrote:
> 
>>> To be honest I'm not terribly happy about
>>> pushing this into the regmap core code; if we start needing to do stuff
>>> like this we should expose the bus interface.
> 
>> That's certainly an option, but I'd really like to use the regmap caching
>> stuff in here.  These things can have quite a few registers that other than
>> their weird read / write quirks look much like any other register based
>> device. (particularly ignoring the burst reads but they tend to apply
>> to volatile registers only so caching is probably irrelevant).
> 
>> At the moment, the only hooks AFAIKS to allow this are at the bus level.
>> I'm not sure where else they could go. (I haven't actually looked much
>> at the cache code yet though).
> 
> Well, if there's not hooks they could be added if it's not too painful.
> However I'm not convinced that's sane.
> 
>> Actually I may have misunderstood, do you mean expose the bus interface
>> within regmap or just not use regmap at all?
> 
> Either.  Or either expose marshalling or add configuration for the
> marshalling depending on what's being done, I'm not convinced you need
> to be working at SPI level at all.
I'm a little unclear what you mean by 'marshalling'.  Could you expand
on how that might work?
> 
>>>> +static struct regmap_bus regmap_spi_adi = {
>>>> +	.write = regmap_spi_write,
>>>> +	.read = regmap_spi_read,
>>>> +};
> 
>>> You want to implement the gather write too if you can.
> 
>> Doesn't really exist other than by linearising them into a series
>> of calls to the write function. (assuming I understand what those
>> functions are doing right!)
> 
> If you build up a series of SPI transfers you just need to control when
> /CS gets bounced appropriately.
Sure, might be worth doing.  To be honest I don't really care about writes
as these are write rarely read often devices other than single register
writes to clear interrupts (even that is often a read).
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 2/6] regmap: Add a magic bus type to handle quirks of analog devices ADIS sensors.

[Jonathan Cameron]

On 09/07/11 19:44, Mark Brown wrote:
> On Wed, Sep 07, 2011 at 07:50:07PM +0100, Jonathan Cameron wrote:
>> On 09/07/11 19:32, Mark Brown wrote:
> 
>>> Either.  Or either expose marshalling or add configuration for the
>>> marshalling depending on what's being done, I'm not convinced you need
>>> to be working at SPI level at all.
> 
>> I'm a little unclear what you mean by 'marshalling'.  Could you expand
>> on how that might work?
> 
> Marshalling is the process of formatting data for transmit and parsing
> it once it's been read in.
Ah, got you now.  Yes, something in there might work or as you say add some
hooks to do it outside.