From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752033AbcAPNRH (ORCPT ); Sat, 16 Jan 2016 08:17:07 -0500 Received: from argos.tuxed.org ([176.9.87.227]:42079 "EHLO argos.tuxed.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751563AbcAPNRF (ORCPT ); Sat, 16 Jan 2016 08:17:05 -0500 Subject: Re: [PATCH 2/2] iio: light: opt3001: enable operation w/o IRQ To: Jonathan Cameron , Peter Meerwald-Stadler References: <1452622180-12728-1-git-send-email-mail@alexanderkoch.net> <1452622180-12728-3-git-send-email-mail@alexanderkoch.net> <56955F43.90705@alexanderkoch.net> <569A3D06.80701@kernel.org> <569A3D48.8040703@kernel.org> Cc: 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 From: Alexander Koch Message-ID: <569A42C4.7010508@alexanderkoch.net> Date: Sat, 16 Jan 2016 14:16:52 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <569A3D48.8040703@kernel.org> 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 Am 16.01.2016 um 13:53 schrieb Jonathan Cameron: > On 16/01/16 12:52, Jonathan Cameron wrote: >> On 12/01/16 20:17, Alexander Koch wrote: >>> 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. >> Bit fields are often less efficient as the compiler has to separate them out >> using shifts and masks. Also from a space point of view the data structure >> will be considerably padded anyway for a couple of reasons: >> 1) It contains u32 fields so will at least be padded to a multiple of u32. >> 2) Memory allocations may well be a good bit larger depending on exact >> sizes vs the blob levels available in the memory allocator. >> >> Basic rule of thumb - keep things simple and let the compiler do the work. >> So a bool is suitable here. >> >>> >>> 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? >> Presumably ;) >>> 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. >> Now perhaps the 'right' way to do this would be have been a precursor patch >> removing the define rather than lumping what is an an connected change (in >> many ways) in here. Overall I agree the change is worthwhile and trivial. >> As Peter said, it's a matter of taste! We both happen to disagree with him >> on this point. > > ps. Should have said that other than the bit field vs bool change, the patch > looks good to me. Okay then, so will send a v2 of the patch that includes this bool change, shortly. While I'm at it, maybe I should include a second refactoring commit that changes the other bitfield members of the opt3001-struct that are used as bool as well - namely 'ok_to_ignore_lock' and 'result_ready'. I hope this is okay. Best regards Alex > > Jonathan >>> >>> >> > Best regards >>> >>> lynix >>> >> >> -- >> 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 >> >