From: Manuel Stahl <manuel.stahl@iis.fraunhofer.de>
To: Jonathan Cameron <jic23@cam.ac.uk>
Cc: linux-iio@vger.kernel.org, Michael.Hennerich@analog.com,
Robin.Getz@analog.com
Subject: Re: [RFC PATCH 0/6] staging:iio:imu driver merges, fixes and new features.
Date: Wed, 22 Sep 2010 10:47:59 +0200 [thread overview]
Message-ID: <4C99C2BF.5030107@iis.fraunhofer.de> (raw)
In-Reply-To: <1284217100-2469-1-git-send-email-jic23@cam.ac.uk>
[-- Attachment #1: Type: text/plain, Size: 4916 bytes --]
Hi Jonathan,
the merge still has severe problems for adis16400. Ring buffer doesn't
read elements (bytes_per_datum is always 8) but the buffer contains
zeros for the elements and a valid timestamp.
The temp attribute has a different address for adis300 and adis400, so
we need a separate entry here.
The attached patch fixes a few problems, but the ring buffer still does
not work.
Am 11.09.2010 16:58, schrieb Jonathan Cameron:
> The first 4 are repeat postings but are now complete (oops).
>
> Cleaning up this driver set has been on my todo list for a while
> and I've finally had a bit of time to see what could be done to
> reduce the huge amount of repeat code found in the 3 IMU drivers.
>
> This is an RFC for two reasons. Firstly I haven't tested this as
> fully yet and will do so before merging. I will do a whole
> lot more testing on the adis16350 that I have. Coverage of the
> adis16300 and adis16400 would be great if anyone can do so.
> I also haven't done full testing on all the build combinations yet.
> (there is at least one known issue to clean up!)
> Secondly the event patch in particularly has some elements not seen
> elsewhere before that I would like people to consider.
>
> Right now, only the adis16350 and adis16360 famillies are supported
> by the event support patch. I'll add support for the other parts
> at a later date unless someone else beats me to it (hint!). Note
> this patch now actually contains the adis16350_event.c file that
> was missing previously.
>
> There is one effective change to all but the adis16400 ABIs.
> When we originally proposed the [m]_<type>_<modifier>_en
> attributes for scan modes I very carefully stated that the index
> [m] for a given device would not necessarily cover all values between
> 0 and the highest present. It was exactly this possible driver
> merge that motivated that arguement. By allowing indexes to be
> missed, we can have single drivers supporting various subsets of
> sensor elements. Here the devices are such that some interleaving
> is possible. This reduces the storage required for the event code,
> so I have done it where possible. Note the addition
> of new devices to this driver in future may change this again!
>
> As ever, all comments welcome. Any tested-bys with a part number
> would be particularly welcome.
>
> Should be trivial to add the adis16367 and adis16385 to this driver.
> Does anyone have one of these parts to test?
>
> Jonathan Cameron (6):
> staging:iio:adis16350 add non burst buffer fill and fix burst logic
> staging:iio:adis16350 move datardy trigger to straight interrupt.
> staging:iio:adis16350 Add optional event support
> staging:iio:adis16350 add missing registration of temp_offset attr
> staging:iio:adis16300 merge into adis16350 driver
> staging:iio:adis16400 merge into adis16350 driver
>
> drivers/staging/iio/adc/adc.h | 8 +-
> drivers/staging/iio/gyro/gyro.h | 31 +-
> drivers/staging/iio/imu/Kconfig | 30 +-
> drivers/staging/iio/imu/Makefile | 9 +-
> drivers/staging/iio/imu/adis16300.h | 184 -------
> drivers/staging/iio/imu/adis16300_core.c | 756 ---------------------------
> drivers/staging/iio/imu/adis16300_ring.c | 220 --------
> drivers/staging/iio/imu/adis16300_trigger.c | 125 -----
> drivers/staging/iio/imu/adis16350.h | 117 ++++-
> drivers/staging/iio/imu/adis16350_core.c | 287 +++++++++--
> drivers/staging/iio/imu/adis16350_event.c | 499 ++++++++++++++++++
> drivers/staging/iio/imu/adis16350_ring.c | 167 +++++-
> drivers/staging/iio/imu/adis16350_trigger.c | 40 +-
> drivers/staging/iio/imu/adis16400.h | 208 --------
> drivers/staging/iio/imu/adis16400_core.c | 752 --------------------------
> drivers/staging/iio/imu/adis16400_ring.c | 231 --------
> drivers/staging/iio/sysfs.h | 17 +-
> 17 files changed, 1067 insertions(+), 2614 deletions(-)
> delete mode 100644 drivers/staging/iio/imu/adis16300.h
> delete mode 100644 drivers/staging/iio/imu/adis16300_core.c
> delete mode 100644 drivers/staging/iio/imu/adis16300_ring.c
> delete mode 100644 drivers/staging/iio/imu/adis16300_trigger.c
> create mode 100644 drivers/staging/iio/imu/adis16350_event.c
> delete mode 100644 drivers/staging/iio/imu/adis16400.h
> delete mode 100644 drivers/staging/iio/imu/adis16400_core.c
> delete mode 100644 drivers/staging/iio/imu/adis16400_ring.c
>
--
Dipl.-Inf. Manuel Stahl
Fraunhofer-Institut für Integrierte Schaltungen IIS
- Leistungsoptimierte Systeme -
Nordostpark 93 Telefon +49 (0)911/58061-6419
90411 Nürnberg Fax +49 (0)911/58061-6398
http://www.iis.fraunhofer.de manuel.stahl@iis.fraunhofer.de
[-- Attachment #2: adis16350_merge_fix.patch --]
[-- Type: text/plain, Size: 4576 bytes --]
diff --git a/drivers/staging/iio/imu/adis16350.h b/drivers/staging/iio/imu/adis16350.h
index ab98d6c..a37f199 100644
--- a/drivers/staging/iio/imu/adis16350.h
+++ b/drivers/staging/iio/imu/adis16350.h
@@ -22,7 +22,8 @@
#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 ADIS16300_TEMP_OUT 0x10 /* Temperature output */
#define ADIS16300_XINCLI_OUT 0x12 /* X-axis inclinometer output measurement */
@@ -52,6 +53,14 @@
#define ADIS16350_ALM_CTRL 0x2E /* Alarm control */
#define ADIS16350_AUX_DAC 0x30 /* Auxiliary DAC data */
+#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 ADIS16350_ERROR_ACTIVE (1<<14)
#define ADIS16350_NEW_DATA (1<<15)
diff --git a/drivers/staging/iio/imu/adis16350_core.c b/drivers/staging/iio/imu/adis16350_core.c
index d8f76c6..b921f1d 100644
--- a/drivers/staging/iio/imu/adis16350_core.c
+++ b/drivers/staging/iio/imu/adis16350_core.c
@@ -484,7 +484,7 @@ static int adis16350_initial_setup(struct adis16350_state *st)
/* use high spi speed if possible */
ret = adis16350_spi_read_reg_16(dev, ADIS16350_SMPL_PRD, &smp_prd);
if (!ret && (smp_prd & ADIS16350_SMPL_PRD_DIV_MASK) < 0x0A) {
- st->us->max_speed_hz = ADIS16350_SPI_SLOW;
+ st->us->max_speed_hz = ADIS16350_SPI_FAST;
spi_setup(st->us);
}
@@ -680,29 +680,32 @@ static IIO_DEV_ATTR_MAGN_Y(adis16350_read_14bit_signed,
static IIO_DEV_ATTR_MAGN_Z(adis16350_read_14bit_signed,
ADIS16400_ZMAGN_OUT);
static IIO_CONST_ATTR(magn_scale, "0.0005 Gs");
+static IIO_DEVICE_ATTR(temp0_raw, S_IRUGO, adis16350_read_12bit_unsigned,
+ NULL, ADIS16300_TEMP_OUT);
+
static struct attribute *adis16400_attributes[] = {
- &iio_dev_attr_gyro_x_calibbias.dev_attr.attr,
- &iio_dev_attr_gyro_y_calibbias.dev_attr.attr,
- &iio_dev_attr_gyro_z_calibbias.dev_attr.attr,
- &iio_dev_attr_accel_x_calibbias.dev_attr.attr,
- &iio_dev_attr_accel_y_calibbias.dev_attr.attr,
- &iio_dev_attr_accel_z_calibbias.dev_attr.attr,
&iio_dev_attr_in_supply_raw.dev_attr.attr,
&iio_const_attr_in_supply_scale.dev_attr.attr,
+ &iio_dev_attr_gyro_scale.dev_attr.attr,
&iio_dev_attr_gyro_x_raw.dev_attr.attr,
&iio_dev_attr_gyro_y_raw.dev_attr.attr,
&iio_dev_attr_gyro_z_raw.dev_attr.attr,
- &iio_dev_attr_gyro_scale.dev_attr.attr,
+ &iio_dev_attr_gyro_x_calibbias.dev_attr.attr,
+ &iio_dev_attr_gyro_y_calibbias.dev_attr.attr,
+ &iio_dev_attr_gyro_z_calibbias.dev_attr.attr,
+ &iio_dev_attr_accel_scale.dev_attr.attr,
&iio_dev_attr_accel_x_raw.dev_attr.attr,
&iio_dev_attr_accel_y_raw.dev_attr.attr,
&iio_dev_attr_accel_z_raw.dev_attr.attr,
- &iio_dev_attr_accel_scale.dev_attr.attr,
+ &iio_dev_attr_accel_x_calibbias.dev_attr.attr,
+ &iio_dev_attr_accel_y_calibbias.dev_attr.attr,
+ &iio_dev_attr_accel_z_calibbias.dev_attr.attr,
+ &iio_const_attr_magn_scale.dev_attr.attr,
&iio_dev_attr_magn_x_raw.dev_attr.attr,
&iio_dev_attr_magn_y_raw.dev_attr.attr,
&iio_dev_attr_magn_z_raw.dev_attr.attr,
- &iio_const_attr_magn_scale.dev_attr.attr,
- &iio_dev_attr_temp_raw.dev_attr.attr,
+ &iio_dev_attr_temp0_raw.dev_attr.attr,
&iio_const_attr_temp_scale.dev_attr.attr,
&iio_const_attr_temp_offset.dev_attr.attr,
&iio_dev_attr_in0_raw.dev_attr.attr,
@@ -833,7 +836,7 @@ static int __devinit adis16350_probe(struct spi_device *spi)
}
st->indio_dev->dev.parent = &spi->dev;
- st->indio_dev->attrs = &adis16350_attribute_group;
+ st->indio_dev->attrs = st->variant->attribute_group;
st->indio_dev->dev_data = (void *)(st);
st->indio_dev->driver_module = THIS_MODULE;
st->indio_dev->modes = INDIO_DIRECT_MODE;
@@ -967,5 +970,6 @@ static __exit void adis16350_exit(void)
module_exit(adis16350_exit);
MODULE_AUTHOR("Barry Song <21cnbao@gmail.com>");
+MODULE_AUTHOR("Manuel Stahl <manuel.stahl@iis.fraunhofer.de>");
MODULE_DESCRIPTION("Analog Devices ADIS16350/54/55/60/62/64/65 IMU SPI driver");
MODULE_LICENSE("GPL v2");
next prev parent reply other threads:[~2010-09-22 8:47 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-09-11 14:58 [RFC PATCH 0/6] staging:iio:imu driver merges, fixes and new features Jonathan Cameron
2010-09-11 14:58 ` [PATCH 1/6] staging:iio:adis16350 add non burst buffer fill and fix burst logic Jonathan Cameron
2010-09-11 14:58 ` [PATCH 2/6] staging:iio:adis16350 move datardy trigger to straight interrupt Jonathan Cameron
2010-09-11 14:58 ` [PATCH 3/6] staging:iio:adis16350 Add optional event support Jonathan Cameron
2010-09-11 14:58 ` [PATCH 4/6] staging:iio:adis16350 add missing registration of temp_offset attr Jonathan Cameron
2010-09-11 14:58 ` [PATCH 5/6] staging:iio:adis16300 merge into adis16350 driver Jonathan Cameron
2010-09-18 16:06 ` Jonathan Cameron
2010-09-11 14:58 ` [PATCH 6/6] staging:iio:adis16400 " Jonathan Cameron
2010-09-11 15:05 ` Jonathan Cameron
2010-09-22 8:47 ` Manuel Stahl [this message]
2010-09-22 10:12 ` [RFC PATCH 0/6] staging:iio:imu driver merges, fixes and new features Jonathan Cameron
2010-09-22 10:17 ` Jonathan Cameron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4C99C2BF.5030107@iis.fraunhofer.de \
--to=manuel.stahl@iis.fraunhofer.de \
--cc=Michael.Hennerich@analog.com \
--cc=Robin.Getz@analog.com \
--cc=jic23@cam.ac.uk \
--cc=linux-iio@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox