[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <dfc205f9-172f-3407-272f-dcef8cacfc4d@codeaurora.org>
Date: Wed, 26 Oct 2016 16:15:42 +0530
From: "Phani A, Rama Krishna" <rphani@...eaurora.org>
To: Sricharan <sricharan@...eaurora.org>, linux-iio@...r.kernel.org,
jic23@...nel.org
Cc: linux-arm-msm@...r.kernel.org, smohanad@...eaurora.org,
mgautam@...eaurora.org, 'Hartmut Knaack' <knaack.h@....de>,
'Lars-Peter Clausen' <lars@...afoo.de>,
'Peter Meerwald-Stadler' <pmeerw@...erw.net>,
'Julia Lawall' <Julia.Lawall@...6.fr>,
'open list' <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V1]iio: adc: spmi-vadc: Changes to support different
scaling
Hi Sricharan,
On 26-Oct-16 3:47 PM, Sricharan wrote:
> Hi Ramakrishna,
>
> [snip..]
>
>>>> + u32 i = 0;
>>>> +
>>>> + if (!pts)
>>>> + return -EINVAL;
>>>> +
>>>> + /* Check if table is descending or ascending */
>>>> + if (tablesize > 1) {
>>>> + if (pts[0].x < pts[1].x)
>>>> + descending = 0;
>>>> + }
>>>> +
>>>> + while (i < tablesize) {
>>>> + if ((descending == 1) && (pts[i].x < input)) {
>>>
>>> Just if (descending) instead of (descending == 1) and so on for the below as well
>>
>> Will change in next patch.
>>
>>>
>>>> + /* table entry is less than measured*/
>>>> + /* value and table is descending, stop */
>>>> + break;
>>>> + } else if ((descending == 0) &&
>>>> + (pts[i].x > input)) {
>>>> + /* table entry is greater than measured*/
>>>> + /*value and table is ascending, stop */
>>>> + break;
>>>> + }
>>>> + i++;
>>>> + }
>>>> +
>>>> + if (i == 0) {
>>>> + *output = pts[0].y;
>>>> + } else if (i == tablesize) {
>>>> + *output = pts[tablesize - 1].y;
>>>> + } else {
>>>> + /* result is between search_index and search_index-1 */
>>>> + /* interpolate linearly */
>>>> + *output = (((s32)((pts[i].y - pts[i - 1].y) *
>>>> + (input - pts[i - 1].x)) /
>>>> + (pts[i].x - pts[i - 1].x)) +
>>>> + pts[i - 1].y);
>>>> + }
>>>
>>> hmm, so for descending, input - pts[i -1].x is negative and
>>> we are adding that to pts[i-1].y, is that correct ?
>>
>> The formula used is to interpolate between two points using linear
>> interpolation.
>
> Right, agree. my question can be ignored.
>
> [snip..]
>
>>>> #define VADC_CHAN_TEMP(_dname, _pre) \
>>>> - VADC_CHAN(_dname, IIO_TEMP, BIT(IIO_CHAN_INFO_PROCESSED), _pre) \
>>>> + VADC_CHAN(_dname, IIO_TEMP, \
>>>> + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_PROCESSED), \
>>>> + _pre) \
>>>>
>>>> #define VADC_CHAN_VOLT(_dname, _pre) \
>>>> - VADC_CHAN(_dname, IIO_VOLTAGE, \
>>>> - BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), \
>>>> + VADC_CHAN(_dname, IIO_VOLTAGE, \
>>>> + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_PROCESSED),\
>>>> _pre) \
>>>>
>>> For this and the below changes to VADC_CHAN_VOLT to TEMP, why is that done ?
>>> Now both macros are setting the same flags.
>>
>> For Voltage channels IIO_VOLTAGE is needed where as for Temperature
>> channels IIO_TEMP is needed.
>>
>>>
>>>> /*
>>>> @@ -637,12 +811,11 @@ struct vadc_channels {
>>>> VADC_CHAN_TEMP(DIE_TEMP, 0)
>>>> VADC_CHAN_VOLT(REF_625MV, 0)
>>>> VADC_CHAN_VOLT(REF_1250MV, 0)
>>>> - VADC_CHAN_VOLT(CHG_TEMP, 0)
>>>> + VADC_CHAN_TEMP(CHG_TEMP, 0)
>>>> VADC_CHAN_VOLT(SPARE1, 0)
>>>> VADC_CHAN_VOLT(SPARE2, 0)
>>>> VADC_CHAN_VOLT(GND_REF, 0)
>>>> VADC_CHAN_VOLT(VDD_VADC, 0)
>>>> -
>
> And also looks like the deletion of these and below
> new lines are unnecessary ?
Agree, Will retain these new lines in next patch V2.
>
> Regards,
> Sricharan
>
Thanks,
Ramakrishna
Powered by blists - more mailing lists