From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932403AbcA3Q1f (ORCPT ); Sat, 30 Jan 2016 11:27:35 -0500 Received: from saturn.retrosnub.co.uk ([178.18.118.26]:53672 "EHLO saturn.retrosnub.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932239AbcA3Q1e (ORCPT ); Sat, 30 Jan 2016 11:27:34 -0500 Subject: Re: [PATCH v2 3/3] iio: light: opt3001: enable operation w/o IRQ To: Andreas Dannenberg , Alexander Koch References: <1452960878-1727-1-git-send-email-mail@alexanderkoch.net> <1452960878-1727-4-git-send-email-mail@alexanderkoch.net> <20160129180125.GC7960@LTA0797059A> Cc: knaack.h@gmx.de, lars@metafoo.de, pmeerw@pmeerw.net, mhornung.linux@gmail.com, balbi@ti.com, fengguang.wu@intel.com, linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org From: Jonathan Cameron Message-ID: <56ACE472.8050107@kernel.org> Date: Sat, 30 Jan 2016 16:27:30 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 MIME-Version: 1.0 In-Reply-To: <20160129180125.GC7960@LTA0797059A> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 29/01/16 18:01, Andreas Dannenberg wrote: > On Sat, Jan 16, 2016 at 05:14:38PM +0100, 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. >> >> Signed-off-by: Alexander Koch >> Signed-off-by: Michael Hornung > > Tested-by: Andreas Dannenberg > > I tested both interrupt mode and non-interrupt mode using a BeagleBone > Black. Works like a champ. Also the hard-coded integration "wait" times > look good. > Thanks, I have added your tested-by to the patches. Jonathan > > -- > Andreas Dannenberg > Texas Instruments Inc > > >> --- >> drivers/iio/light/opt3001.c | 137 ++++++++++++++++++++++++++++++-------------- >> 1 file changed, 95 insertions(+), 42 deletions(-) >> >> diff --git a/drivers/iio/light/opt3001.c b/drivers/iio/light/opt3001.c >> index b05c484..2a2340d 100644 >> --- a/drivers/iio/light/opt3001.c >> +++ b/drivers/iio/light/opt3001.c >> @@ -70,9 +70,12 @@ >> >> /* >> * Time to wait for conversion result to be ready. The device datasheet >> - * worst-case max value is 880ms. Add some slack to be on the safe side. >> + * sect. 6.5 states results are ready after total integration time plus 3ms. >> + * This results in worst-case max values of 113ms or 883ms, respectively. >> + * Add some slack to be on the safe side. >> */ >> -#define OPT3001_RESULT_READY_TIMEOUT msecs_to_jiffies(1000) >> +#define OPT3001_RESULT_READY_SHORT 150 >> +#define OPT3001_RESULT_READY_LONG 1000 >> >> struct opt3001 { >> struct i2c_client *client; >> @@ -92,6 +95,8 @@ struct opt3001 { >> >> u8 high_thresh_exp; >> u8 low_thresh_exp; >> + >> + bool use_irq; >> }; >> >> struct opt3001_scale { >> @@ -230,26 +235,30 @@ static int opt3001_get_lux(struct opt3001 *opt, int *val, int *val2) >> u16 reg; >> u8 exponent; >> u16 value; >> + long timeout; >> >> - /* >> - * Enable the end-of-conversion interrupt mechanism. Note that doing >> - * so will overwrite the low-level limit value however we will restore >> - * this value later on. >> - */ >> - ret = i2c_smbus_write_word_swapped(opt->client, OPT3001_LOW_LIMIT, >> - OPT3001_LOW_LIMIT_EOC_ENABLE); >> - if (ret < 0) { >> - dev_err(opt->dev, "failed to write register %02x\n", >> - OPT3001_LOW_LIMIT); >> - return ret; >> + if (opt->use_irq) { >> + /* >> + * Enable the end-of-conversion interrupt mechanism. Note that >> + * doing so will overwrite the low-level limit value however we >> + * will restore this value later on. >> + */ >> + ret = i2c_smbus_write_word_swapped(opt->client, >> + OPT3001_LOW_LIMIT, >> + OPT3001_LOW_LIMIT_EOC_ENABLE); >> + if (ret < 0) { >> + dev_err(opt->dev, "failed to write register %02x\n", >> + OPT3001_LOW_LIMIT); >> + return ret; >> + } >> + >> + /* Allow IRQ to access the device despite lock being set */ >> + opt->ok_to_ignore_lock = true; >> } >> >> - /* Reset data-ready indicator flag (will be set in the IRQ routine) */ >> + /* Reset data-ready indicator flag */ >> opt->result_ready = false; >> >> - /* Allow IRQ to access the device despite lock being set */ >> - opt->ok_to_ignore_lock = true; >> - >> /* Configure for single-conversion mode and start a new conversion */ >> ret = i2c_smbus_read_word_swapped(opt->client, OPT3001_CONFIGURATION); >> if (ret < 0) { >> @@ -269,32 +278,69 @@ static int opt3001_get_lux(struct opt3001 *opt, int *val, int *val2) >> goto err; >> } >> >> - /* Wait for the IRQ to indicate the conversion is complete */ >> - ret = wait_event_timeout(opt->result_ready_queue, opt->result_ready, >> - OPT3001_RESULT_READY_TIMEOUT); >> + if (opt->use_irq) { >> + /* Wait for the IRQ to indicate the conversion is complete */ >> + ret = wait_event_timeout(opt->result_ready_queue, >> + opt->result_ready, >> + msecs_to_jiffies(OPT3001_RESULT_READY_LONG)); >> + } else { >> + /* Sleep for result ready time */ >> + timeout = (opt->int_time == OPT3001_INT_TIME_SHORT) ? >> + OPT3001_RESULT_READY_SHORT : OPT3001_RESULT_READY_LONG; >> + msleep(timeout); >> + >> + /* Check result ready flag */ >> + ret = i2c_smbus_read_word_swapped(opt->client, >> + OPT3001_CONFIGURATION); >> + if (ret < 0) { >> + dev_err(opt->dev, "failed to read register %02x\n", >> + OPT3001_CONFIGURATION); >> + goto err; >> + } >> + >> + if (!(ret & OPT3001_CONFIGURATION_CRF)) { >> + ret = -ETIMEDOUT; >> + goto err; >> + } >> + >> + /* Obtain value */ >> + ret = i2c_smbus_read_word_swapped(opt->client, OPT3001_RESULT); >> + if (ret < 0) { >> + dev_err(opt->dev, "failed to read register %02x\n", >> + OPT3001_RESULT); >> + goto err; >> + } >> + opt->result = ret; >> + opt->result_ready = true; >> + } >> >> err: >> - /* Disallow IRQ to access the device while lock is active */ >> - opt->ok_to_ignore_lock = false; >> + if (opt->use_irq) >> + /* Disallow IRQ to access the device while lock is active */ >> + opt->ok_to_ignore_lock = false; >> >> if (ret == 0) >> return -ETIMEDOUT; >> else if (ret < 0) >> return ret; >> >> - /* >> - * Disable the end-of-conversion interrupt mechanism by restoring the >> - * low-level limit value (clearing OPT3001_LOW_LIMIT_EOC_ENABLE). Note >> - * that selectively clearing those enable bits would affect the actual >> - * limit value due to bit-overlap and therefore can't be done. >> - */ >> - value = (opt->low_thresh_exp << 12) | opt->low_thresh_mantissa; >> - ret = i2c_smbus_write_word_swapped(opt->client, OPT3001_LOW_LIMIT, >> - value); >> - if (ret < 0) { >> - dev_err(opt->dev, "failed to write register %02x\n", >> - OPT3001_LOW_LIMIT); >> - return ret; >> + if (opt->use_irq) { >> + /* >> + * Disable the end-of-conversion interrupt mechanism by >> + * restoring the low-level limit value (clearing >> + * OPT3001_LOW_LIMIT_EOC_ENABLE). Note that selectively clearing >> + * those enable bits would affect the actual limit value due to >> + * bit-overlap and therefore can't be done. >> + */ >> + value = (opt->low_thresh_exp << 12) | opt->low_thresh_mantissa; >> + ret = i2c_smbus_write_word_swapped(opt->client, >> + OPT3001_LOW_LIMIT, >> + value); >> + if (ret < 0) { >> + dev_err(opt->dev, "failed to write register %02x\n", >> + OPT3001_LOW_LIMIT); >> + return ret; >> + } >> } >> >> exponent = OPT3001_REG_EXPONENT(opt->result); >> @@ -736,12 +782,18 @@ static int opt3001_probe(struct i2c_client *client, >> return ret; >> } >> >> - ret = request_threaded_irq(irq, NULL, opt3001_irq, >> - IRQF_TRIGGER_FALLING | IRQF_ONESHOT, >> - "opt3001", iio); >> - if (ret) { >> - dev_err(dev, "failed to request IRQ #%d\n", irq); >> - return ret; >> + /* Make use of INT pin only if valid IRQ no. is given */ >> + if (irq > 0) { >> + ret = request_threaded_irq(irq, NULL, opt3001_irq, >> + IRQF_TRIGGER_FALLING | IRQF_ONESHOT, >> + "opt3001", iio); >> + if (ret) { >> + dev_err(dev, "failed to request IRQ #%d\n", irq); >> + return ret; >> + } >> + opt->use_irq = true; >> + } else { >> + dev_info(opt->dev, "enabling interrupt-less operation\n"); >> } >> >> return 0; >> @@ -754,7 +806,8 @@ static int opt3001_remove(struct i2c_client *client) >> int ret; >> u16 reg; >> >> - free_irq(client->irq, iio); >> + if (opt->use_irq) >> + free_irq(client->irq, iio); >> >> ret = i2c_smbus_read_word_swapped(opt->client, OPT3001_CONFIGURATION); >> if (ret < 0) { >> -- >> 2.7.0 >> > -- > 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 >