* [RFC PATCH 0/6] Using regmap with ADIS devices.
2011-09-07 16:10 ` Blockers on IIO usage of regmap Jonathan Cameron
@ 2011-09-07 16:19 ` Jonathan Cameron
2011-09-07 16:19 ` [PATCH 1/6] SPI: add ability to say we want a cs change after every transfer Jonathan Cameron
` (6 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Jonathan Cameron @ 2011-09-07 16:19 UTC (permalink / raw)
To: broonie; +Cc: linux-kernel, Michael.Hennerich, linux-iio, 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
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCH 1/6] SPI: add ability to say we want a cs change after every transfer.
2011-09-07 16:10 ` Blockers on IIO usage of regmap Jonathan Cameron
2011-09-07 16:19 ` [RFC PATCH 0/6] Using regmap with ADIS devices Jonathan Cameron
@ 2011-09-07 16:19 ` Jonathan Cameron
2011-09-07 17:35 ` Mark Brown
2011-09-07 16:19 ` [PATCH 2/6] regmap: Add a magic bus type to handle quirks of analog devices ADIS sensors Jonathan Cameron
` (5 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Jonathan Cameron @ 2011-09-07 16:19 UTC (permalink / raw)
To: broonie; +Cc: linux-kernel, Michael.Hennerich, linux-iio, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 1/6] SPI: add ability to say we want a cs change after every transfer.
2011-09-07 16:19 ` [PATCH 1/6] SPI: add ability to say we want a cs change after every transfer Jonathan Cameron
@ 2011-09-07 17:35 ` Mark Brown
2011-09-07 18:14 ` Jonathan Cameron
0 siblings, 1 reply; 21+ messages in thread
From: Mark Brown @ 2011-09-07 17:35 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-kernel, Michael.Hennerich, linux-iio
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/6] SPI: add ability to say we want a cs change after every transfer.
2011-09-07 17:35 ` Mark Brown
@ 2011-09-07 18:14 ` Jonathan Cameron
2011-09-07 18:12 ` Mark Brown
0 siblings, 1 reply; 21+ messages in thread
From: Jonathan Cameron @ 2011-09-07 18:14 UTC (permalink / raw)
To: Mark Brown; +Cc: linux-kernel, Michael.Hennerich, linux-iio
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
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/6] SPI: add ability to say we want a cs change after every transfer.
2011-09-07 18:14 ` Jonathan Cameron
@ 2011-09-07 18:12 ` Mark Brown
2011-09-07 18:28 ` Jonathan Cameron
0 siblings, 1 reply; 21+ messages in thread
From: Mark Brown @ 2011-09-07 18:12 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-kernel, Michael.Hennerich, linux-iio
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/6] SPI: add ability to say we want a cs change after every transfer.
2011-09-07 18:12 ` Mark Brown
@ 2011-09-07 18:28 ` Jonathan Cameron
0 siblings, 0 replies; 21+ messages in thread
From: Jonathan Cameron @ 2011-09-07 18:28 UTC (permalink / raw)
To: Mark Brown; +Cc: linux-kernel, Michael.Hennerich, linux-iio
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
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/6] regmap: Add a magic bus type to handle quirks of analog devices ADIS sensors.
2011-09-07 16:10 ` Blockers on IIO usage of regmap Jonathan Cameron
2011-09-07 16:19 ` [RFC PATCH 0/6] Using regmap with ADIS devices Jonathan Cameron
2011-09-07 16:19 ` [PATCH 1/6] SPI: add ability to say we want a cs change after every transfer Jonathan Cameron
@ 2011-09-07 16:19 ` Jonathan Cameron
2011-09-07 17:47 ` Mark Brown
2011-09-07 16:19 ` [PATCH 3/6] staging:iio:imu: adis16400 partial conversion to regmap Jonathan Cameron
` (4 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Jonathan Cameron @ 2011-09-07 16:19 UTC (permalink / raw)
To: broonie; +Cc: linux-kernel, Michael.Hennerich, linux-iio, 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, ®map_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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 2/6] regmap: Add a magic bus type to handle quirks of analog devices ADIS sensors.
2011-09-07 16:19 ` [PATCH 2/6] regmap: Add a magic bus type to handle quirks of analog devices ADIS sensors Jonathan Cameron
@ 2011-09-07 17:47 ` Mark Brown
2011-09-07 18:26 ` Jonathan Cameron
0 siblings, 1 reply; 21+ messages in thread
From: Mark Brown @ 2011-09-07 17:47 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-kernel, Michael.Hennerich, linux-iio
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.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 2/6] regmap: Add a magic bus type to handle quirks of analog devices ADIS sensors.
2011-09-07 17:47 ` Mark Brown
@ 2011-09-07 18:26 ` Jonathan Cameron
2011-09-07 18:32 ` Mark Brown
0 siblings, 1 reply; 21+ messages in thread
From: Jonathan Cameron @ 2011-09-07 18:26 UTC (permalink / raw)
To: Mark Brown; +Cc: linux-kernel, Michael.Hennerich, linux-iio
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
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 2/6] regmap: Add a magic bus type to handle quirks of analog devices ADIS sensors.
2011-09-07 18:26 ` Jonathan Cameron
@ 2011-09-07 18:32 ` Mark Brown
2011-09-07 18:50 ` Jonathan Cameron
0 siblings, 1 reply; 21+ messages in thread
From: Mark Brown @ 2011-09-07 18:32 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-kernel, Michael.Hennerich, linux-iio
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.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 2/6] regmap: Add a magic bus type to handle quirks of analog devices ADIS sensors.
2011-09-07 18:32 ` Mark Brown
@ 2011-09-07 18:50 ` Jonathan Cameron
2011-09-07 18:44 ` Mark Brown
0 siblings, 1 reply; 21+ messages in thread
From: Jonathan Cameron @ 2011-09-07 18:50 UTC (permalink / raw)
To: Mark Brown; +Cc: linux-kernel, Michael.Hennerich, linux-iio
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
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 2/6] regmap: Add a magic bus type to handle quirks of analog devices ADIS sensors.
2011-09-07 18:50 ` Jonathan Cameron
@ 2011-09-07 18:44 ` Mark Brown
2011-09-07 19:14 ` Jonathan Cameron
0 siblings, 1 reply; 21+ messages in thread
From: Mark Brown @ 2011-09-07 18:44 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-kernel, Michael.Hennerich, linux-iio
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/6] regmap: Add a magic bus type to handle quirks of analog devices ADIS sensors.
2011-09-07 18:44 ` Mark Brown
@ 2011-09-07 19:14 ` Jonathan Cameron
0 siblings, 0 replies; 21+ messages in thread
From: Jonathan Cameron @ 2011-09-07 19:14 UTC (permalink / raw)
To: Mark Brown; +Cc: linux-kernel, Michael.Hennerich, linux-iio
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/6] staging:iio:imu: adis16400 partial conversion to regmap.
2011-09-07 16:10 ` Blockers on IIO usage of regmap Jonathan Cameron
` (2 preceding siblings ...)
2011-09-07 16:19 ` [PATCH 2/6] regmap: Add a magic bus type to handle quirks of analog devices ADIS sensors Jonathan Cameron
@ 2011-09-07 16:19 ` Jonathan Cameron
2011-09-07 16:23 ` Jonathan Cameron
2011-09-07 16:19 ` [PATCH 4/6] regmap-spi-adi + staging:iio:imu:adis16400 halve register addresses Jonathan Cameron
` (3 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Jonathan Cameron @ 2011-09-07 16:19 UTC (permalink / raw)
To: broonie; +Cc: linux-kernel, Michael.Hennerich, linux-iio, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 3/6] staging:iio:imu: adis16400 partial conversion to regmap.
2011-09-07 16:19 ` [PATCH 3/6] staging:iio:imu: adis16400 partial conversion to regmap Jonathan Cameron
@ 2011-09-07 16:23 ` Jonathan Cameron
0 siblings, 0 replies; 21+ messages in thread
From: Jonathan Cameron @ 2011-09-07 16:23 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: broonie, linux-kernel, Michael.Hennerich, linux-iio
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;
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 4/6] regmap-spi-adi + staging:iio:imu:adis16400 halve register addresses
2011-09-07 16:10 ` Blockers on IIO usage of regmap Jonathan Cameron
` (3 preceding siblings ...)
2011-09-07 16:19 ` [PATCH 3/6] staging:iio:imu: adis16400 partial conversion to regmap Jonathan Cameron
@ 2011-09-07 16:19 ` Jonathan Cameron
2011-09-07 16:19 ` [PATCH 5/6] regmap-spi-adi generalize regmap_spi_read Jonathan Cameron
` (2 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Jonathan Cameron @ 2011-09-07 16:19 UTC (permalink / raw)
To: broonie; +Cc: linux-kernel, Michael.Hennerich, linux-iio, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 5/6] regmap-spi-adi generalize regmap_spi_read.
2011-09-07 16:10 ` Blockers on IIO usage of regmap Jonathan Cameron
` (4 preceding siblings ...)
2011-09-07 16:19 ` [PATCH 4/6] regmap-spi-adi + staging:iio:imu:adis16400 halve register addresses Jonathan Cameron
@ 2011-09-07 16:19 ` Jonathan Cameron
2011-09-07 16:19 ` [PATCH 6/6] staging:iio:imu:adis16400 make use of regmap bulk read capabilities Jonathan Cameron
2011-09-07 17:57 ` Blockers on IIO usage of regmap Mark Brown
7 siblings, 0 replies; 21+ messages in thread
From: Jonathan Cameron @ 2011-09-07 16:19 UTC (permalink / raw)
To: broonie; +Cc: linux-kernel, Michael.Hennerich, linux-iio, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 6/6] staging:iio:imu:adis16400 make use of regmap bulk read capabilities
2011-09-07 16:10 ` Blockers on IIO usage of regmap Jonathan Cameron
` (5 preceding siblings ...)
2011-09-07 16:19 ` [PATCH 5/6] regmap-spi-adi generalize regmap_spi_read Jonathan Cameron
@ 2011-09-07 16:19 ` Jonathan Cameron
2011-09-07 17:57 ` Blockers on IIO usage of regmap Mark Brown
7 siblings, 0 replies; 21+ messages in thread
From: Jonathan Cameron @ 2011-09-07 16:19 UTC (permalink / raw)
To: broonie; +Cc: linux-kernel, Michael.Hennerich, linux-iio, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: Blockers on IIO usage of regmap.
2011-09-07 16:10 ` Blockers on IIO usage of regmap Jonathan Cameron
` (6 preceding siblings ...)
2011-09-07 16:19 ` [PATCH 6/6] staging:iio:imu:adis16400 make use of regmap bulk read capabilities Jonathan Cameron
@ 2011-09-07 17:57 ` Mark Brown
2011-09-07 18:12 ` Jonathan Cameron
7 siblings, 1 reply; 21+ messages in thread
From: Mark Brown @ 2011-09-07 17:57 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: LKML, Hennerich, Michael, linux-iio@vger.kernel.org
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.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: Blockers on IIO usage of regmap.
2011-09-07 17:57 ` Blockers on IIO usage of regmap Mark Brown
@ 2011-09-07 18:12 ` Jonathan Cameron
0 siblings, 0 replies; 21+ messages in thread
From: Jonathan Cameron @ 2011-09-07 18:12 UTC (permalink / raw)
To: Mark Brown; +Cc: LKML, Hennerich, Michael, linux-iio@vger.kernel.org
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
>
^ permalink raw reply [flat|nested] 21+ messages in thread