From: jic23@kernel.org (Jonathan Cameron)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 4/4] hwmon: iio: add label for channels read by iio_hwmon
Date: Wed, 20 Jul 2016 15:49:33 +0100 [thread overview]
Message-ID: <1e7e369d-21c1-b05d-63c6-bf6551e268ae@kernel.org> (raw)
In-Reply-To: <578DCED5.6080408@free-electrons.com>
On 19/07/16 07:55, Quentin Schulz wrote:
> On 18/07/2016 14:24, Jonathan Cameron wrote:
>> On 15/07/16 10:59, Quentin Schulz wrote:
>>> Currently, iio_hwmon only exposes values of the IIO channels it can read
>>> but no label by channel is exposed.
>>>
>>> This adds exposition of sysfs files containing label for IIO channels it
>>> can read based on extended_name field of the iio_chan_spec of the channel.
>>> If the extended_name field is empty, the sysfs file is not created by
>>> iio_hwmon.
>> Hmm. This is not the intent of extended name at all. That exists to add
>> a small amount of information to an constructed IIO channel name.
>> Typically it's used to indicate physically wired stuff like:
>>
>> in_voltage0_vdd_raw for cases where that channel of the ADC is hard wired
>> to the vdd. In this particular case the use might actually work as the
>> vdd makes it clear it's a voltage - in general that's not the case.
>>
>> Use of extended_name at all in IIO is only done after extensive review.
>> It adds nasty custom ABI to a device, so the gain has to be considerable
>> to use it.
>>
>> When I read your original suggestion of adding labels, I was expecting the
>> use of datasheet_name. That has the advantage of being well defined by
>> the datasheet (if not it should not be provided) + not being used in
>> the construction of the IIO channel related attributes. However, that
>> may still not correspond well to the expected labelling in hwmon.
>
> Good to know for extend_name use cases. While doing further testing, I
> noticed the extend_name is also appended to the sysfs filename.. which
> is definitely not what we want.
>
> I've checked for datasheet_name and it is only used to be compared to
> adc_channel_label from iio_map structure. Same for adc_channel_label
> (which has to be the same as datasheet_name of the iio_chan_spec it is
> linked to). So I could use this instead of extend_name to put a label on
> a channel.
> However, I thought of it to be more a way to identify the hardware in
> the datasheet more than giving users a hint on what it is. That's what
> "git grep adc_channel_label" told me. It's definitely better to use
> datasheet_name over extend_name for channel labeling but I don't know if
> it's really the good variable to use for labeling? In my understanding
> of datasheet_name, in my case it would be more "temp_gpadc" than "SoC
> temperature", that's what I mean.
If we are going to do this I think we need a new field in iio_chan_spec
for it. The problem as ever is going to be that we'll end up with
'fuzzy' ABI which we can't change - even if we end up with spelling
mistakes sneaking through review - or entirely incorrect labels.
>
>> Thinking more on this, the label is going to often be a function of how
>> the board is wired up... Perhaps it should be a characteristic of the
>> channel_map (hence from DT or similar) rather than part of the IIO driver
>> itself?
>
> Hmm.. I would not put a property in the DT only for labeling.
It's an odd one because in many cases the map (which is effectively
representing a wire) is the only relevant place to have such a label.
Can see what you mean about not putting things in DT which are basically
'help'!
>
> [...]
>
prev parent reply other threads:[~2016-07-20 14:49 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-15 9:59 [PATCH v2 0/4] add support for Allwinner SoCs ADC Quentin Schulz
2016-07-15 9:59 ` [PATCH v2 1/4] hwmon: iio_hwmon: defer probe when no channel is found Quentin Schulz
2016-07-16 17:00 ` [v2,1/4] " Guenter Roeck
2016-07-18 10:02 ` Maxime Ripard
2016-07-18 13:29 ` Guenter Roeck
2016-07-15 9:59 ` [PATCH v2 2/4] iio: adc: add support for Allwinner SoCs ADC Quentin Schulz
2016-07-18 12:57 ` Maxime Ripard
2016-07-19 9:04 ` Quentin Schulz
2016-07-19 12:40 ` Maxime Ripard
2016-07-18 13:18 ` Jonathan Cameron
2016-07-19 8:33 ` Quentin Schulz
2016-07-20 14:57 ` Jonathan Cameron
2016-07-21 12:15 ` Quentin Schulz
2016-07-23 6:37 ` Jonathan Cameron
2016-07-20 12:37 ` Quentin Schulz
2016-07-20 14:15 ` Crt Mori
2016-07-20 14:59 ` Jonathan Cameron
2016-07-15 9:59 ` [PATCH v2 3/4] mfd: " Quentin Schulz
2016-07-18 13:02 ` Maxime Ripard
2016-07-19 12:04 ` Quentin Schulz
2016-07-18 13:25 ` Jonathan Cameron
2016-07-19 7:31 ` Lee Jones
2016-07-20 15:01 ` Jonathan Cameron
2016-07-21 12:12 ` Lee Jones
2016-07-21 20:08 ` Maxime Ripard
2016-07-22 13:55 ` Lee Jones
2016-07-23 6:42 ` Jonathan Cameron
2016-07-25 9:55 ` Maxime Ripard
2016-07-19 8:35 ` Quentin Schulz
2016-07-15 9:59 ` [PATCH v2 4/4] hwmon: iio: add label for channels read by iio_hwmon Quentin Schulz
2016-07-15 14:03 ` Guenter Roeck
2016-07-15 14:36 ` Quentin Schulz
2016-07-16 2:53 ` Guenter Roeck
2016-07-18 12:24 ` Jonathan Cameron
2016-07-19 6:55 ` Quentin Schulz
2016-07-20 14:49 ` Jonathan Cameron [this message]
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=1e7e369d-21c1-b05d-63c6-bf6551e268ae@kernel.org \
--to=jic23@kernel.org \
--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).