Linux IIO development
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Lars-Peter Clausen <lars@metafoo.de>
Cc: linux-iio@vger.kernel.org
Subject: Re: [PATCH 0/3]  staging:iio:adis16130 bits and bobs.
Date: Sun, 14 Apr 2013 18:56:33 +0100	[thread overview]
Message-ID: <516AEDD1.9050206@kernel.org> (raw)
In-Reply-To: <516AED37.6030204@kernel.org>

On 04/14/2013 06:53 PM, Jonathan Cameron wrote:
> On 04/14/2013 01:20 PM, Lars-Peter Clausen wrote:
>> On 04/14/2013 01:23 PM, Jonathan Cameron wrote:
>>> Hi All,
>>>
>>> Had a few bored moments so thought I'd take a quick look at this driver.
>>> The read function is rather odd to say the least, Lars/Michael could one of
>>> you take a quick look at this.  Right now it does an spi sync that I think
>>> should read the data and follows it with an additional read.  I can't
>>> immediately see what the read is for.
>>
>> Yea I noticed that too, I think that read was introduced by accident during
>> some refactoring. See
>> https://github.com/lclausen-adi/linux-2.6/commit/1889f3a5d291ad57a5faed83652c465de29d740f
> oops.
>>
>> Unfortunately I couldn't get my hands on a adis16030 board yet to test those
>> changes otherwise I'd already submitted them.
> I did wonder why this one had slipped through your net.
> 
> Even untested, I'd be tempted to apply your patch.  What is there is pretty
> obviously garbage whereas yours is probably right!  I guess it depends on time
> scales for getting the part.
> 
>>
>>>
>>> Also note the introduction of IIO_INT_PLUS_PICO which is going to be common
>>> with 24 bit plus devices.  For now I've ignored the write case as this driver
>>> doesn't support it, but it will be needed if for example 24/16 bit options
>>> are both supported for this driver.
>>>
>>> Note to my mind this device is far enough away from the other adis parts
>>> in interface to justify it's own driver.
>>
>> Yes.
>>
>>>
>>> Also could someone check my scale/offset calcs are right. It's Sunday
>>> morning and I'm not feeling all that awake ;)
>>>
>>
>> For temp scale you seem to be off by a factor of 1000000 (well or maybe
>> I'm), otherwise I got the same.
> oops :)  Divide vs multiply...
> 
> Will post a v2 in a few mins for what we had here and let you handle
> the interesting read as you like.

Actually seeing as you have similar patches queued up in your tree I'll
leave it all for you and take the lazy way out ;)

*continues looking about in the tree you referenced to see what other nuggets
you are working on :)*

Jonathan
>>
>>> Thanks,
>>>
>>> Jonathan 
>>>
>>> Jonathan Cameron (3):
>>>   iio:Add an IIO_VAL_INTO_PLUS_PICO return type for read_raw callbacks.
>>>   staging:iio:gyro:adis16130 drop unused list.h header.
>>>   staging:iio:gyro:adis16130 add offset and scale info mask elements.
>>>
>>>  drivers/iio/industrialio-core.c           |  5 +++
>>>  drivers/staging/iio/gyro/adis16130_core.c | 58 +++++++++++++++++++++++++------
>>>  include/linux/iio/types.h                 |  1 +
>>>  3 files changed, 53 insertions(+), 11 deletions(-)
>>>
>>
> --
> 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
> 

  reply	other threads:[~2013-04-14 17:56 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-14 11:23 [PATCH 0/3] staging:iio:adis16130 bits and bobs Jonathan Cameron
2013-04-14 11:23 ` [PATCH 1/3] iio:Add an IIO_VAL_INTO_PLUS_PICO return type for read_raw callbacks Jonathan Cameron
2013-04-14 12:22   ` Lars-Peter Clausen
2013-04-14 11:23 ` [PATCH 2/3] staging:iio:gyro:adis16130 drop unused list.h header Jonathan Cameron
2013-04-14 11:23 ` [PATCH 3/3] staging:iio:gyro:adis16130 add offset and scale info mask elements Jonathan Cameron
2013-04-14 12:20 ` [PATCH 0/3] staging:iio:adis16130 bits and bobs Lars-Peter Clausen
2013-04-14 17:53   ` Jonathan Cameron
2013-04-14 17:56     ` Jonathan Cameron [this message]
2013-04-14 18:31       ` Lars-Peter Clausen

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=516AEDD1.9050206@kernel.org \
    --to=jic23@kernel.org \
    --cc=lars@metafoo.de \
    --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