From: lars@metafoo.de (Lars-Peter Clausen)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/4] hwmon: Add a simple driver to read the MXS SoC temperature
Date: Fri, 28 Jun 2013 16:18:01 +0200 [thread overview]
Message-ID: <51CD9B19.90407@metafoo.de> (raw)
In-Reply-To: <51CC91E8.50605@free-electrons.com>
On 06/27/2013 09:26 PM, Alexandre Belloni wrote:
> Hi,
>
> On 27/06/2013 16:27, Guenter Roeck wrote:
>> On Thu, Jun 27, 2013 at 11:17:32AM +0200, Maxime Ripard wrote:
>>> On Wed, Jun 26, 2013 at 07:39:27AM -0700, Guenter Roeck wrote:
>>>> On Wed, Jun 26, 2013 at 10:51:12AM +0200, Alexandre Belloni wrote:
>>>>> The low resolution ADC of the mxs is able to read an internal temperature
>>>>> sensor, expose that using hwmon.
>>>>>
>>>>> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>>>>> ---
>>>> Wouldn't it make more sense to use iio-hwmon and improve it if necessary ?
>>> Actually, I wonder if we should not just put the hwmon driver
>>> capabilities directly into the mxs-lradc driver, just like it's already
>>> been done in this driver for the touchscreen support.
>>>
>>> The probing of this hwmon driver doesn't really belong to the DT, it's
>>> not really realistic to probe it from the machine definition, and it
>>> really is the IP that is wired that way.
>>>
>> Merging iio-hwmon functionality into an adc driver seems just as bad
>> (or even worse) as copying it into a new driver.
>>
>> If the lradc driver knows that the ADC channels are temperature sensors, it
>> should register them with the iio subsystem as IIO_TEMP type. Then you should
>> be able to use iio_hwmon as is.
>
> They are already registered as IIO_TEMP but only implement read_raw. Also,
>
> iio_hwmon_read_val() is using iio_read_channel_processed() and that will
> basically only read one of the 2 channels. As I documented, you actually
> need to read both channel 8 and channel 9 and then compute the value in
> Kelvins. I'm not sure how you want me to do that in the current framework.
What are these two channels actually measuring? Is the value of a single
channel meaningful on it's own? If not it might make sense to update the IIO
driver to just have one temperature channel.
- Lars
next prev parent reply other threads:[~2013-06-28 14:18 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-26 8:51 [PATCH 0/4] Add an hwmon driver for the mxs soc internal sensor Alexandre Belloni
2013-06-26 8:51 ` [PATCH 1/4] arm: mxs: Add #io-channel-cells property to lradc Alexandre Belloni
2013-06-26 8:51 ` [PATCH 2/4] arm: mxs: add lradc to cfa10036 Alexandre Belloni
2013-06-26 8:51 ` [PATCH 3/4] hwmon: Add a simple driver to read the MXS SoC temperature Alexandre Belloni
2013-06-26 14:39 ` Guenter Roeck
2013-06-27 9:17 ` Maxime Ripard
2013-06-27 14:27 ` Guenter Roeck
2013-06-27 19:26 ` Alexandre Belloni
2013-06-28 14:18 ` Lars-Peter Clausen [this message]
2013-06-28 14:50 ` Alexandre Belloni
2013-06-28 15:24 ` Lars-Peter Clausen
2013-06-28 16:35 ` Guenter Roeck
2013-06-26 8:51 ` [PATCH 4/4] arm: mxs: Add mxs internal temp sensor to cfa-10036 Alexandre Belloni
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=51CD9B19.90407@metafoo.de \
--to=lars@metafoo.de \
--cc=linux-arm-kernel@lists.infradead.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).