All of lore.kernel.org
 help / color / mirror / Atom feed
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");

  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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.