Message-ID: <bad05e06-3b37-b435-bfac-962aef36cc97@gmail.com>
Date:   Mon, 13 Mar 2023 14:56:59 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH v3 2/6] iio: light: Add gain-time-scale helpers
Content-Language: en-US, en-GB
To:     Jonathan Cameron <jic23@kernel.org>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc:     Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>,
        Lars-Peter Clausen <lars@metafoo.de>,
        Shreeya Patel <shreeya.patel@collabora.com>,
        Paul Gazzillo <paul@pgazz.com>,
        Dmitry Osipenko <dmitry.osipenko@collabora.com>,
        Zhigang Shi <Zhigang.Shi@liteon.com>,
        linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org
References: <cover.1678093787.git.mazziesaccount@gmail.com>
 <a4cb9a34ca027867ac014ffe93ca7e8245ce263f.1678093787.git.mazziesaccount@gmail.com>
 <ZAXiKfRbsXpHhwAJ@smile.fi.intel.com> <20230312165100.45de0c9b@jic23-huawei>
From:   Matti Vaittinen <mazziesaccount@gmail.com>
In-Reply-To: <20230312165100.45de0c9b@jic23-huawei>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 3/12/23 18:51, Jonathan Cameron wrote:
> On Mon, 6 Mar 2023 14:52:57 +0200
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
>> On Mon, Mar 06, 2023 at 11:17:15AM +0200, Matti Vaittinen wrote:
>>> Some light sensors can adjust both the HW-gain and integration time.
>>> There are cases where adjusting the integration time has similar impact
>>> to the scale of the reported values as gain setting has.
>>>
>>> IIO users do typically expect to handle scale by a single writable 'scale'
>>> entry. Driver should then adjust the gain/time accordingly.
>>>
>>> It however is difficult for a driver to know whether it should change
>>> gain or integration time to meet the requested scale. Usually it is
>>> preferred to have longer integration time which usually improves
>>> accuracy, but there may be use-cases where long measurement times can be
>>> an issue. Thus it can be preferable to allow also changing the
>>> integration time - but mitigate the scale impact by also changing the gain
>>> underneath. Eg, if integration time change doubles the measured values,
>>> the driver can reduce the HW-gain to half.
>>>
>>> The theory of the computations of gain-time-scale is simple. However,
>>> some people (undersigned) got that implemented wrong for more than once.
>>>
>>> Add some gain-time-scale helpers in order to not dublicate errors in all
>>> drivers needing these computations.
>>
>> ...
>>
>>> +/*
>>
>> If it's deliberately not a kernel doc, why to bother to have it looking as one?
>> It's really a provocative to some people who will come with a patches to "fix"
>> this...
> 
> Just make it kernel-doc.
> 

Are you sure...? I don't like the idea of polluting generated docs with 
documentation for this type of tiny internal pieces not usable outside 
this component anyways...

>>
>>> + * iio_gts_get_gain - Convert scale to total gain
>>> + *
>>> + * Internal helper for converting scale to total gain.
>>> + *
>>> + * @max:	Maximum linearized scale. As an example, when scale is created
>>> + *		in magnitude of NANOs and max scale is 64.1 - The linearized
>>> + *		scale is 64 100 000 000.
>>> + * @scale:	Linearized scale to compte the gain for.
>>> + *
>>> + * Return:	(floored) gain corresponding to the scale. -EINVAL if scale
>>> + *		is invalid.
>>> + */
> 
>> ...
>>
>>> +EXPORT_SYMBOL_NS_GPL(iio_gts_total_gain_to_scale, IIO_GTS_HELPER);
>>
>> I would say _HELPER part is too much, but fine with me.
> 
> Hmm. I think I like the HELPER bit as separates it from being a driver.
> Of course I might change my mind after a few sleeps.

Ever considered a career as a politician? ;) (No offense intended - and 
feel free to change your mind on this. I don't expect this to be done 
tomorrow)

> 
>>> +++ b/drivers/iio/light/iio-gts-helper.h
>>
>> Is it _only_ for a Light type of sensors?
> 
> I'd move it up a directory and allow for other users.
> 

Ok. I'll do the move for the next version.

Yours,
	-- Matti

> Jonathan

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~