From: Laxman Dewangan <ldewangan@nvidia.com>
To: Jonathan Cameron <jic23@jic23.retrosnub.co.uk>,
Jonathan Cameron <jic23@kernel.org>, <robh@kernel.org>,
<edubezval@gmail.com>, <rui.zhang@intel.com>,
<mark.rutland@arm.com>, <pawel.moll@arm.com>,
<ijc+devicetree@hellion.org.uk>
Cc: <linux-pm@vger.kernel.org>, <devicetree@vger.kernel.org>,
<linux-kernel@vger.kernel.org>, <linux-iio@vger.kernel.org>
Subject: Re: [PATCH V3 2/2] thermal: generic-adc: Add ADC based thermal sensor driver
Date: Mon, 18 Apr 2016 23:04:48 +0530 [thread overview]
Message-ID: <57151AB8.7040405@nvidia.com> (raw)
In-Reply-To: <D84D2ED7-81D4-4635-8244-4017D66C6A3B@jic23.retrosnub.co.uk>
On Monday 18 April 2016 11:01 PM, Jonathan Cameron wrote:
>
> On 18 April 2016 17:49:39 BST, Laxman Dewangan <ldewangan@nvidia.com> wrote:
>> On Sunday 17 April 2016 04:24 PM, Jonathan Cameron wrote:
>>> On 14/04/16 15:41, Laxman Dewangan wrote:
>>> +static int gadc_thermal_read_channel(struct gadc_thermal_info *gti,
>> int *val)
>>> +{
>>> + int ret;
>>> +
>>> + ret = iio_read_channel_processed(gti->channel, val);
>>> + if (ret < 0)
>>> + ret = iio_read_channel_raw(gti->channel, val);
>>> Is this case actually useful given it means the scaling of the adc
>>> isn't known?
>>>
>>> I suppose you might have defined the table in terms of raw readings,
>>> but then when someone comes along and 'fixes' the ADC driver to
>> output
>>> it's scale your table will be wrong.
>>>
>> Yes, that may be possible if someone just move the implementation of
>> processed read to raw read.
>> I assumed that some of adc driver implemented as raw and some of
>> implemented as processed and so fallback.
>>
>> However, if adc driver has processed implementation then it should not
>> move to raw and deprecate the processed.
>>
>> It seems raw as default should be better option. We can have two option
>> now:
>>
>> - Support raw only, not to processed.
>>
>> - Or support the raw as default and processed as the optional from DT.
>> if (!processed)
>> read_raw()
>> else
>> read_processed()
>>
>>
>> Your opinion?
> Processed only. It will compute the right value if raw and scale are provided by the
> device (which they should be for an ADC). The read_processed function does
> the maths if needed.
>
> The only time devices should
> supply raw without scale is if their is no direct transform ( e.g. an infrared
> intensity measure where only known transform involves combining it with
> another signal) or their is an external unknown (e.g. proximity sensors where
> you have to know what they were close to in order to know the scaling!)
>
> If there is a conventional ADC driver not providing either processed directly or
> both raw and scale let us know and we will fix it!
>
Thanks, I will recycle this patch to use processed only.
WARNING: multiple messages have this Message-ID (diff)
From: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
To: Jonathan Cameron
<jic23-tko9wxEg+fIOOJlXag/Snyp2UmYkHbXO@public.gmane.org>,
Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
edubezval-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
rui.zhang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
mark.rutland-5wv7dgnIgG8@public.gmane.org,
pawel.moll-5wv7dgnIgG8@public.gmane.org,
ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org
Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH V3 2/2] thermal: generic-adc: Add ADC based thermal sensor driver
Date: Mon, 18 Apr 2016 23:04:48 +0530 [thread overview]
Message-ID: <57151AB8.7040405@nvidia.com> (raw)
In-Reply-To: <D84D2ED7-81D4-4635-8244-4017D66C6A3B-tko9wxEg+fIOOJlXag/Snyp2UmYkHbXO@public.gmane.org>
On Monday 18 April 2016 11:01 PM, Jonathan Cameron wrote:
>
> On 18 April 2016 17:49:39 BST, Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>> On Sunday 17 April 2016 04:24 PM, Jonathan Cameron wrote:
>>> On 14/04/16 15:41, Laxman Dewangan wrote:
>>> +static int gadc_thermal_read_channel(struct gadc_thermal_info *gti,
>> int *val)
>>> +{
>>> + int ret;
>>> +
>>> + ret = iio_read_channel_processed(gti->channel, val);
>>> + if (ret < 0)
>>> + ret = iio_read_channel_raw(gti->channel, val);
>>> Is this case actually useful given it means the scaling of the adc
>>> isn't known?
>>>
>>> I suppose you might have defined the table in terms of raw readings,
>>> but then when someone comes along and 'fixes' the ADC driver to
>> output
>>> it's scale your table will be wrong.
>>>
>> Yes, that may be possible if someone just move the implementation of
>> processed read to raw read.
>> I assumed that some of adc driver implemented as raw and some of
>> implemented as processed and so fallback.
>>
>> However, if adc driver has processed implementation then it should not
>> move to raw and deprecate the processed.
>>
>> It seems raw as default should be better option. We can have two option
>> now:
>>
>> - Support raw only, not to processed.
>>
>> - Or support the raw as default and processed as the optional from DT.
>> if (!processed)
>> read_raw()
>> else
>> read_processed()
>>
>>
>> Your opinion?
> Processed only. It will compute the right value if raw and scale are provided by the
> device (which they should be for an ADC). The read_processed function does
> the maths if needed.
>
> The only time devices should
> supply raw without scale is if their is no direct transform ( e.g. an infrared
> intensity measure where only known transform involves combining it with
> another signal) or their is an external unknown (e.g. proximity sensors where
> you have to know what they were close to in order to know the scaling!)
>
> If there is a conventional ADC driver not providing either processed directly or
> both raw and scale let us know and we will fix it!
>
Thanks, I will recycle this patch to use processed only.
next prev parent reply other threads:[~2016-04-18 17:46 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-14 14:41 [PATCH V3 1/2] thermal: generic-adc: Add DT binding for ADC based thermal sensor Laxman Dewangan
2016-04-14 14:41 ` Laxman Dewangan
2016-04-14 14:41 ` [PATCH V3 2/2] thermal: generic-adc: Add ADC based thermal sensor driver Laxman Dewangan
2016-04-14 14:41 ` Laxman Dewangan
2016-04-17 10:54 ` Jonathan Cameron
2016-04-17 10:54 ` Jonathan Cameron
2016-04-18 16:49 ` Laxman Dewangan
2016-04-18 16:49 ` Laxman Dewangan
2016-04-18 17:31 ` Jonathan Cameron
2016-04-18 17:34 ` Laxman Dewangan [this message]
2016-04-18 17:34 ` Laxman Dewangan
2016-04-14 17:35 ` [PATCH V3 1/2] thermal: generic-adc: Add DT binding for ADC based thermal sensor Rob Herring
2016-04-14 17:35 ` Rob Herring
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=57151AB8.7040405@nvidia.com \
--to=ldewangan@nvidia.com \
--cc=devicetree@vger.kernel.org \
--cc=edubezval@gmail.com \
--cc=ijc+devicetree@hellion.org.uk \
--cc=jic23@jic23.retrosnub.co.uk \
--cc=jic23@kernel.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=pawel.moll@arm.com \
--cc=robh@kernel.org \
--cc=rui.zhang@intel.com \
/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.