All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexander Koch <mail@alexanderkoch.net>
To: Peter Meerwald-Stadler <pmeerw@pmeerw.net>
Cc: jic23@kernel.org, knaack.h@gmx.de, lars@metafoo.de,
	mhornung.linux@gmail.com, dannenberg@ti.com, balbi@ti.com,
	linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] iio: light: opt3001: enable operation w/o IRQ
Date: Tue, 12 Jan 2016 21:17:07 +0100	[thread overview]
Message-ID: <56955F43.90705@alexanderkoch.net> (raw)
In-Reply-To: <alpine.DEB.2.02.1601122024340.10606@pmeerw.net>

Am 12.01.2016 um 20:27 schrieb Peter Meerwald-Stadler:
> On Tue, 12 Jan 2016, Alexander Koch wrote:
> 
>> Enable operation of the TI OPT3001 light sensor without having an
>> interrupt line available to connect the INT pin to.
>>
>> In this operation mode, we issue a conversion request and simply wait
>> for the conversion time available as timeout value, determined from
>> integration time configuration and the worst-case time given in the data
>> sheet (sect. 6.5, table on p. 5):
>>
>>   short integration time (100ms): 110ms + 3ms = 113ms
>>    long integration time (800ms): 880ms + 3ms = 883ms
>>
>> This change is transparent as behaviour defaults to using the interrupt
>> method if an interrupt no. is configured via device tree. Interrupt-less
>> operation mode is performed when no valid interrupt no. is given.
> 
> looks good, I'd rather use a bool for use_irq and the msecs_to_jiffies() 
> call moved from the #define to the code (which is not strictly necessary 
> for the patch) -- matter of taste

Thanks - actually this is my first patch, so positive feedback much
appreciated!

Concerning the bool for 'use_irq': I first had it that way but then
opted for the bit field of length 1 as I wasn't sure whether bool would
get optimized to the same level by the compiler.

I'm a bit irritated by your comment concerning the msecs_to_jiffies()
call, as my patch indeed moves this call from the #define to the code.
Did you mean it the other way round, then?
My reason to move it was that I need to work with microseconds for the
IRQ-less operation mode, and jiffies are only required in one place for
the IRQ mode.


Best regards

lynix

  reply	other threads:[~2016-01-12 20:17 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-12 18:09 [PATCH 0/2] iio: light: opt3001: Enable operation w/o IRQ Alexander Koch
2016-01-12 18:09 ` [PATCH 1/2] iio: light: opt3001: extract int. time constants Alexander Koch
2016-01-12 18:09 ` [PATCH 2/2] iio: light: opt3001: enable operation w/o IRQ Alexander Koch
2016-01-12 19:27   ` Peter Meerwald-Stadler
2016-01-12 20:17     ` Alexander Koch [this message]
2016-01-16 12:52       ` Jonathan Cameron
2016-01-16 12:53         ` Jonathan Cameron
2016-01-16 13:16           ` Alexander Koch

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=56955F43.90705@alexanderkoch.net \
    --to=mail@alexanderkoch.net \
    --cc=balbi@ti.com \
    --cc=dannenberg@ti.com \
    --cc=jic23@kernel.org \
    --cc=knaack.h@gmx.de \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhornung.linux@gmail.com \
    --cc=pmeerw@pmeerw.net \
    /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.