From: Jonathan Cameron <jic23@cam.ac.uk>
To: "Hennerich, Michael" <Michael.Hennerich@analog.com>
Cc: "linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
"device-drivers-devel@blackfin.uclinux.org"
<device-drivers-devel@blackfin.uclinux.org>,
Drivers <Drivers@analog.com>
Subject: Re: [PATCH 05/12] iio: ad7291: don't swab results twice and introduce more register defines
Date: Wed, 31 Aug 2011 13:01:59 +0100 [thread overview]
Message-ID: <4E5E22B7.70108@cam.ac.uk> (raw)
In-Reply-To: <544AC56F16B56944AEC3BD4E3D59177146E6F63983@LIMKCMBX1.ad.analog.com>
On 08/31/11 12:49, Hennerich, Michael wrote:
> Jonathan Cameron wrote on 2011-08-31:
>> On 08/31/11 11:57, michael.hennerich@analog.com wrote:
>>> From: Michael Hennerich <michael.hennerich@analog.com>
>>>
>> oops. my bad on the double swab I think. Anyhow all looks sane an good.
>> That fixme with the reverse register order is odd. Guessing you are
>> chasing that down with the hardware guys?
>
> If you like you can remove the FIXME.
> I checked the behavior on hardware, and it seems to be a datasheet errata.
cool. Seemed pretty implausible!
>
>>>
>>> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com> merged
>>> ---
>>> drivers/staging/iio/adc/ad7291.c | 90 ++++++++++++++++++++++++----
>>> --------- 1 files changed, 58 insertions(+), 32 deletions(-)
>>> diff --git a/drivers/staging/iio/adc/ad7291.c
>>> b/drivers/staging/iio/adc/ad7291.c index 382ca3e..2fafca7 100644 ---
>>> a/drivers/staging/iio/adc/ad7291.c +++
>>> b/drivers/staging/iio/adc/ad7291.c @@ -33,18 +33,43 @@
>>> /*
>>> * AD7291 registers definition
>>> */
>>> -#define AD7291_COMMAND 0
>>> -#define AD7291_VOLTAGE 1
>>> -#define AD7291_T_SENSE 2
>>> -#define AD7291_T_AVERAGE 3
>>> -#define AD7291_VOLTAGE_LIMIT_BASE 4
>>> -#define AD7291_VOLTAGE_LIMIT_COUNT 8
>>> +#define AD7291_COMMAND 0x00
>>> +#define AD7291_VOLTAGE 0x01
>>> +#define AD7291_T_SENSE 0x02
>>> +#define AD7291_T_AVERAGE 0x03
>>> +#define AD7291_CH0_DATA_HIGH 0x04
>>> +#define AD7291_CH0_DATA_LOW 0x05
>>> +#define AD7291_CH0_HYST 0x06
>>> +#define AD7291_CH1_DATA_HIGH 0x07
>>> +#define AD7291_CH1_DATA_LOW 0x08
>>> +#define AD7291_CH1_HYST 0x09
>>> +#define AD7291_CH2_DATA_HIGH 0x0A
>>> +#define AD7291_CH2_DATA_LOW 0x0B
>>> +#define AD7291_CH2_HYST 0x0C
>>> +#define AD7291_CH3_DATA_HIGH 0x0D
>>> +#define AD7291_CH3_DATA_LOW 0x0E
>>> +#define AD7291_CH3_HYST 0x0F
>>> +#define AD7291_CH4_DATA_HIGH 0x10
>>> +#define AD7291_CH4_DATA_LOW 0x11
>>> +#define AD7291_CH4_HYST 0x12
>>> +#define AD7291_CH5_DATA_HIGH 0x13
>>> +#define AD7291_CH5_DATA_LOW 0x14
>>> +#define AD7291_CH5_HYST 0x15
>>> +#define AD7291_CH6_DATA_HIGH 0x16
>>> +#define AD7291_CH6_DATA_LOW 0x17
>>> +#define AD7291_CH6_HYST 0x18
>>> +#define AD7291_CH7_DATA_HIGH 0x19
>>> +#define AD7291_CH7_DATA_LOW 0x1A
>>> +#define AD7291_CH7_HYST 0x2B
>>> #define AD7291_T_SENSE_HIGH 0x1C
>>> #define AD7291_T_SENSE_LOW 0x1D
>>> #define AD7291_T_SENSE_HYST 0x1E
>>> #define AD7291_VOLTAGE_ALERT_STATUS 0x1F
>>> #define AD7291_T_ALERT_STATUS 0x20
>>> +#define AD7291_VOLTAGE_LIMIT_COUNT 8
>>> +
>>> +
>>> /*
>>> * AD7291 command
>>> */
>>> @@ -197,7 +222,7 @@ static inline ssize_t ad7291_show_hyst(struct
>> device *dev,
>>> if (ret < 0)
>>> return ret;
>>> - return sprintf(buf, "%d\n", data & 0x0FFF);
>>> + return sprintf(buf, "%d\n", data & AD7291_VALUE_MASK);
>>> }
>>>
>>> static inline ssize_t ad7291_set_hyst(struct device *dev,
>>> @@ -227,28 +252,28 @@ static
>> IIO_DEVICE_ATTR(in_temp0_thresh_both_hyst_raw,
>>> AD7291_T_SENSE_HYST);
>>> static IIO_DEVICE_ATTR(in_voltage0_thresh_both_hyst_raw,
>>> S_IRUGO | S_IWUSR,
>>> - ad7291_show_hyst, ad7291_set_hyst, 0x06);
>>> + ad7291_show_hyst, ad7291_set_hyst, AD7291_CH0_HYST);
>>> static IIO_DEVICE_ATTR(in_voltage1_thresh_both_hyst_raw,
>>> S_IRUGO | S_IWUSR,
>>> - ad7291_show_hyst, ad7291_set_hyst, 0x09);
>>> + ad7291_show_hyst, ad7291_set_hyst, AD7291_CH1_HYST);
>>> static IIO_DEVICE_ATTR(in_voltage2_thresh_both_hyst_raw,
>>> S_IRUGO | S_IWUSR,
>>> - ad7291_show_hyst, ad7291_set_hyst, 0x0C);
>>> + ad7291_show_hyst, ad7291_set_hyst, AD7291_CH2_HYST);
>>> static IIO_DEVICE_ATTR(in_voltage3_thresh_both_hyst_raw,
>>> S_IRUGO | S_IWUSR,
>>> - ad7291_show_hyst, ad7291_set_hyst, 0x0F);
>>> + ad7291_show_hyst, ad7291_set_hyst, AD7291_CH3_HYST);
>>> static IIO_DEVICE_ATTR(in_voltage4_thresh_both_hyst_raw,
>>> S_IRUGO | S_IWUSR,
>>> - ad7291_show_hyst, ad7291_set_hyst, 0x12);
>>> + ad7291_show_hyst, ad7291_set_hyst, AD7291_CH4_HYST);
>>> static IIO_DEVICE_ATTR(in_voltage5_thresh_both_hyst_raw,
>>> S_IRUGO | S_IWUSR,
>>> - ad7291_show_hyst, ad7291_set_hyst, 0x15);
>>> + ad7291_show_hyst, ad7291_set_hyst, AD7291_CH5_HYST);
>>> static IIO_DEVICE_ATTR(in_voltage6_thresh_both_hyst_raw,
>>> S_IRUGO | S_IWUSR,
>>> - ad7291_show_hyst, ad7291_set_hyst, 0x18);
>>> + ad7291_show_hyst, ad7291_set_hyst, AD7291_CH6_HYST);
>>> static IIO_DEVICE_ATTR(in_voltage7_thresh_both_hyst_raw,
>>> S_IRUGO | S_IWUSR,
>>> - ad7291_show_hyst, ad7291_set_hyst, 0x1B);
>>> + ad7291_show_hyst, ad7291_set_hyst, AD7291_CH7_HYST);
>>>
>>> static struct attribute *ad7291_event_attributes[] = {
>>> &iio_dev_attr_in_temp0_thresh_both_hyst_raw.dev_attr.attr,
>>> @@ -265,16 +290,16 @@ static struct attribute
>>> *ad7291_event_attributes[] = {
>>>
>>> /* high / low */
>>> static u8 ad7291_limit_regs[9][2] = {
>>> - { 0x04, 0x05 },
>>> - { 0x07, 0x08 },
>>> - { 0x0A, 0x0B },
>>> - { 0x0E, 0x0D }, /* note reversed order */
>>> - { 0x10, 0x11 },
>>> - { 0x13, 0x14 },
>>> - { 0x16, 0x17 },
>>> - { 0x19, 0x1A },
>>> + { AD7291_CH0_DATA_HIGH, AD7291_CH0_DATA_LOW },
>>> + { AD7291_CH1_DATA_HIGH, AD7291_CH1_DATA_LOW },
>>> + { AD7291_CH2_DATA_HIGH, AD7291_CH2_DATA_LOW },
>>> + { AD7291_CH3_DATA_HIGH, AD7291_CH3_DATA_LOW }, /* FIXME: ? */
>>> + { AD7291_CH4_DATA_HIGH, AD7291_CH4_DATA_LOW },
>>> + { AD7291_CH5_DATA_HIGH, AD7291_CH5_DATA_LOW },
>>> + { AD7291_CH6_DATA_HIGH, AD7291_CH6_DATA_LOW },
>>> + { AD7291_CH7_DATA_HIGH, AD7291_CH7_DATA_LOW },
>>> /* temp */
>>> - { 0x1C, 0x1D },
>>> + { AD7291_T_SENSE_HIGH, AD7291_T_SENSE_LOW },
>>> };
>>>
>>> static int ad7291_read_event_value(struct iio_dev *indio_dev,
>>> @@ -297,7 +322,7 @@ static int ad7291_read_event_value(struct iio_dev
>> *indio_dev,
>>> ret = ad7291_i2c_read(chip, reg, &uval);
>>> if (ret < 0)
>>> return ret;
>>> - *val = swab16(uval) & 0x0FFF;
>>> + *val = uval & AD7291_VALUE_MASK;
>>> return 0;
>>>
>>> case IIO_TEMP:
>>> @@ -308,7 +333,7 @@ static int ad7291_read_event_value(struct iio_dev
>> *indio_dev,
>>> ret = ad7291_i2c_read(chip, reg, &signval);
>>> if (ret < 0)
>>> return ret;
>>> - signval = (s16)((swab16(signval) & 0x0FFF) << 4) >> 4;
>>> + signval = (s16)((signval & AD7291_VALUE_MASK) << 4) >> 4;
>>> *val = signval;
>>> return 0;
>>> default:
>>> @@ -331,9 +356,7 @@ static int ad7291_write_event_value(struct
>> iio_dev *indio_dev,
>>> reg = ad7291_limit_regs[IIO_EVENT_CODE_EXTRACT_NUM(event_code)]
>>> [!(IIO_EVENT_CODE_EXTRACT_DIR(event_code) ==
>>> IIO_EV_DIR_RISING)]; - return ad7291_i2c_write(chip, reg, val); -
>>> case IIO_TEMP: if (val > 2047 || val < -2048) return -EINVAL;
>>> @@ -385,6 +408,7 @@ static int ad7291_write_event_config(struct
>> iio_dev *indio_dev,
>>> * use continuous sampling mode. * Possible to disable temp as well
>>> but that makes single read tricky. */ + switch
>>> (IIO_EVENT_CODE_EXTRACT_TYPE(event_code)) { case IIO_VOLTAGE: if
>>> ((!state) && (chip->c_mask &
>>> @@ -398,7 +422,7 @@ static int ad7291_write_event_config(struct
>> iio_dev *indio_dev,
>>> else
>>> break;
>>> - regval &= 0xFFFE;
>>> + regval &= ~AD7291_AUTOCYCLE;
>>> regval |= ((u16)chip->c_mask << 8);
>>> if (chip->c_mask) /* Enable autocycle? */
>>> regval |= AD7291_AUTOCYCLE;
>>> @@ -455,7 +479,7 @@ static int ad7291_read_raw(struct iio_dev
>> *indio_dev,
>>> mutex_unlock(&chip->state_lock);
>>> return ret;
>>> }
>>> - *val = swab16((u16)ret) & 0x0FFF;
>>> + *val = swab16((u16)ret) & AD7291_VALUE_MASK;
>>> mutex_unlock(&chip->state_lock);
>>> return IIO_VAL_INT;
>>> case IIO_TEMP:
>>> @@ -464,7 +488,8 @@ static int ad7291_read_raw(struct iio_dev
>> *indio_dev,
>>> AD7291_T_SENSE);
>>> if (ret < 0)
>>> return ret;
>>> - signval = (s16)((swab16((u16)ret) & 0x0FFF) << 4) >> 4; + signval
>>> = (s16)((swab16((u16)ret) & + AD7291_VALUE_MASK) << 4) >> 4;
>>> *val = signval;
>>> return IIO_VAL_INT;
>>> default:
>>> @@ -475,7 +500,8 @@ static int ad7291_read_raw(struct iio_dev
>> *indio_dev,
>>> AD7291_T_AVERAGE);
>>> if (ret < 0)
>>> return ret;
>>> - signval = (s16)((swab16((u16)ret) & 0x0FFF) << 4) >> 4; + signval
>>> = (s16)((swab16((u16)ret) & + AD7291_VALUE_MASK) << 4) >> 4;
>>> *val = signval;
>>> return IIO_VAL_INT;
>>> case (1 << IIO_CHAN_INFO_SCALE_SHARED):
>>
>
> Greetings,
> Michael
>
> --
> Analog Devices GmbH Wilhelm-Wagenfeld-Str. 6 80807 Muenchen
> Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
> Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin, Margaret Seif
>
>
>
> --
> 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
>
next prev parent reply other threads:[~2011-08-31 11:53 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-08-31 10:57 [PATCH 01/12] iio: ad7291: fix channel mapping michael.hennerich
2011-08-31 10:57 ` [PATCH 02/12] iio: ad7291: Fix typos, change kconfig description and file header michael.hennerich
2011-08-31 11:18 ` Jonathan Cameron
2011-08-31 10:57 ` [PATCH 03/12] iio: ad7291: Fix AD7291_T_SENSE_MASK michael.hennerich
2011-08-31 11:19 ` Jonathan Cameron
2011-08-31 10:57 ` [PATCH 04/12] iio: ad7291: Add regulator, reference voltage and scale handling michael.hennerich
2011-08-31 11:22 ` Jonathan Cameron
2011-08-31 10:57 ` [PATCH 05/12] iio: ad7291: don't swab results twice and introduce more register defines michael.hennerich
2011-08-31 11:28 ` Jonathan Cameron
2011-08-31 11:49 ` Hennerich, Michael
2011-08-31 12:01 ` Jonathan Cameron [this message]
2011-08-31 10:57 ` [PATCH 06/12] iio: ad7291: fix mask bit generation michael.hennerich
2011-08-31 11:29 ` Jonathan Cameron
2011-08-31 10:57 ` [PATCH 07/12] iio: ad7291: introduce IIO_EVENT_CODE_EXTRACT_CHAN_TYPE and use accordingly michael.hennerich
2011-08-31 11:33 ` Jonathan Cameron
2011-08-31 10:57 ` [PATCH 08/12] iio: ad7291: fix channel mapping for event enables michael.hennerich
2011-08-31 11:34 ` Jonathan Cameron
2011-08-31 10:57 ` [PATCH 09/12] iio: core: fix IIO_EVENT_CODE_EXTRACT_DIR definition michael.hennerich
2011-08-31 11:15 ` Jonathan Cameron
2011-08-31 12:01 ` Jonathan Cameron
2011-08-31 10:57 ` [PATCH 10/12] iio: ad7192: return len and fix out of range checking michael.hennerich
2011-08-31 11:37 ` Jonathan Cameron
2011-08-31 11:47 ` Hennerich, Michael
2011-08-31 11:59 ` Jonathan Cameron
2011-08-31 10:57 ` [PATCH 11/12] iio: ad7291: reset device and setup irq before it is enabled michael.hennerich
2011-08-31 11:39 ` Jonathan Cameron
2011-08-31 10:57 ` [PATCH 12/12] iio: ad7192: add temp_scale attribute, change module description michael.hennerich
2011-08-31 11:52 ` Jonathan Cameron
2011-08-31 11:14 ` [PATCH 01/12] iio: ad7291: fix channel mapping 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=4E5E22B7.70108@cam.ac.uk \
--to=jic23@cam.ac.uk \
--cc=Drivers@analog.com \
--cc=Michael.Hennerich@analog.com \
--cc=device-drivers-devel@blackfin.uclinux.org \
--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;
as well as URLs for NNTP newsgroup(s).