From: Guenter Roeck <linux@roeck-us.net>
To: Lars-Peter Clausen <lars@metafoo.de>,
Jonathan Cameron <jic23@kernel.org>,
Quentin Schulz <quentin.schulz@free-electrons.com>,
jdelvare@suse.com, knaack.h@gmx.de, pmeerw@pmeerw.net,
maxime.ripard@free-electrons.com, wens@csie.org,
lee.jones@linaro.org
Cc: linux-kernel@vger.kernel.org, linux-hwmon@vger.kernel.org,
linux-iio@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
thomas.petazzoni@free-electrons.com,
antoine.tenart@free-electrons.com
Subject: Re: [PATCH 1/3] mfd: add support for Allwinner SoCs ADC
Date: Sun, 3 Jul 2016 10:38:59 -0700 [thread overview]
Message-ID: <57794DB3.2050506@roeck-us.net> (raw)
In-Reply-To: <5779422D.9060207@metafoo.de>
On 07/03/2016 09:49 AM, Lars-Peter Clausen wrote:
> On 07/03/2016 01:17 PM, Jonathan Cameron wrote:
>> On 28/06/16 09:18, Quentin Schulz wrote:
>>> The Allwinner SoCs all have an ADC that can also act as a touchscreen
>>> controller and a thermal sensor. For now, only the ADC and the thermal
>>> sensor drivers are probed by the MFD, the touchscreen controller support
>>> will be added later.
>>>
>>> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
>> The code looks fine to me. The 'controversial' bit of this is listing
>> iio-hwmon as an mfd child to get it to probe as a result of this being
>> present. My immediately thought is that it should be separately
>> described in the devicetree and hence instantiated outside of this driver.
>
> The devicetree is a generic description of the hardware. The iio-hwmon
> bridge is a software component that translates between two Linux specific
> ABIs. In my opinion putting the later in the former is makes no sense, it is
> simply not part of the hardware description.
>
Actually, this is where people get it wrong.
> Its quite terrible that we have the bindings in the first place, but I guess
> we have to keep them considering they are ABI and there are existing users.
> But we should definitely strongly discourage the introduction of new users.
>
I do agree that the _bindings_ are bad.
The ultimate problem is to find bindings which do describe the hardware
in a way that would be acceptable to the devicetree community and is at the
same time useful for software when trying to determine what to do with that
hardware. _This_ is the exceptionally hard problem.
One example would be an adc channel connected to a board voltage. I would assume
that it should be permissible to describe this relationship in a way that can
be _used_ by software to expose that adc channel as voltage, by whatever
means necessary (it be through hwmon or as a regulator or whatever).
Similar, some pin on a chip may be connected to a thermal sensor. I would
assume that it should be permissible to describe that thermal sensor (and its
location) in a way that can be _used_ by software in a meaningful way, either
for it to be reported as hardware monitoring attribute or to be used by the
thermal subsystem as a thermal input channel.
In addition to that, there are various other properties which _do_ describe
the hardware, but are commonly seen as "software". Examples for that would
be voltage or temperature limits (or any other limits, for that matter).
Such limits _are_ part of the hardware description, but are not commonly
accepted as such.
> It is policy whether an application wants to access a device using the IIO
> or hwmon API. As such it must be managed by userspace, this is not something
> that can be done using devicetree nor should it be something that is done on
> a driver by driver basis.
>
Agreed. However, the connections from one chip to another, and the use of a chip
on a board, _is_ part of the hardware description. It is determined by the
schematics as well as the board layout. A well defined hardware description
needs to provide more than "this is an ADC channel" or "this is a thermal sensor".
Guenter
WARNING: multiple messages have this Message-ID (diff)
From: linux@roeck-us.net (Guenter Roeck)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/3] mfd: add support for Allwinner SoCs ADC
Date: Sun, 3 Jul 2016 10:38:59 -0700 [thread overview]
Message-ID: <57794DB3.2050506@roeck-us.net> (raw)
In-Reply-To: <5779422D.9060207@metafoo.de>
On 07/03/2016 09:49 AM, Lars-Peter Clausen wrote:
> On 07/03/2016 01:17 PM, Jonathan Cameron wrote:
>> On 28/06/16 09:18, Quentin Schulz wrote:
>>> The Allwinner SoCs all have an ADC that can also act as a touchscreen
>>> controller and a thermal sensor. For now, only the ADC and the thermal
>>> sensor drivers are probed by the MFD, the touchscreen controller support
>>> will be added later.
>>>
>>> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
>> The code looks fine to me. The 'controversial' bit of this is listing
>> iio-hwmon as an mfd child to get it to probe as a result of this being
>> present. My immediately thought is that it should be separately
>> described in the devicetree and hence instantiated outside of this driver.
>
> The devicetree is a generic description of the hardware. The iio-hwmon
> bridge is a software component that translates between two Linux specific
> ABIs. In my opinion putting the later in the former is makes no sense, it is
> simply not part of the hardware description.
>
Actually, this is where people get it wrong.
> Its quite terrible that we have the bindings in the first place, but I guess
> we have to keep them considering they are ABI and there are existing users.
> But we should definitely strongly discourage the introduction of new users.
>
I do agree that the _bindings_ are bad.
The ultimate problem is to find bindings which do describe the hardware
in a way that would be acceptable to the devicetree community and is at the
same time useful for software when trying to determine what to do with that
hardware. _This_ is the exceptionally hard problem.
One example would be an adc channel connected to a board voltage. I would assume
that it should be permissible to describe this relationship in a way that can
be _used_ by software to expose that adc channel as voltage, by whatever
means necessary (it be through hwmon or as a regulator or whatever).
Similar, some pin on a chip may be connected to a thermal sensor. I would
assume that it should be permissible to describe that thermal sensor (and its
location) in a way that can be _used_ by software in a meaningful way, either
for it to be reported as hardware monitoring attribute or to be used by the
thermal subsystem as a thermal input channel.
In addition to that, there are various other properties which _do_ describe
the hardware, but are commonly seen as "software". Examples for that would
be voltage or temperature limits (or any other limits, for that matter).
Such limits _are_ part of the hardware description, but are not commonly
accepted as such.
> It is policy whether an application wants to access a device using the IIO
> or hwmon API. As such it must be managed by userspace, this is not something
> that can be done using devicetree nor should it be something that is done on
> a driver by driver basis.
>
Agreed. However, the connections from one chip to another, and the use of a chip
on a board, _is_ part of the hardware description. It is determined by the
schematics as well as the board layout. A well defined hardware description
needs to provide more than "this is an ADC channel" or "this is a thermal sensor".
Guenter
next prev parent reply other threads:[~2016-07-03 17:39 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-28 8:45 [PATCH 0/3] add support for Allwinner SoCs ADC Quentin Schulz
2016-06-28 8:45 ` Quentin Schulz
2016-06-28 8:18 ` [PATCH 1/3] mfd: " Quentin Schulz
2016-06-28 8:18 ` Quentin Schulz
2016-06-28 8:30 ` Antoine Tenart
2016-06-28 8:30 ` Antoine Tenart
2016-06-28 8:51 ` Antoine Tenart
2016-06-28 8:51 ` Antoine Tenart
2016-07-03 11:17 ` Jonathan Cameron
2016-07-03 11:17 ` Jonathan Cameron
2016-07-03 16:49 ` Lars-Peter Clausen
2016-07-03 16:49 ` Lars-Peter Clausen
2016-07-03 17:38 ` Guenter Roeck [this message]
2016-07-03 17:38 ` Guenter Roeck
2016-06-28 8:18 ` [PATCH 2/3] iio: adc: " Quentin Schulz
2016-06-28 8:18 ` Quentin Schulz
2016-06-28 8:32 ` Antoine Tenart
2016-06-28 8:32 ` Antoine Tenart
2016-06-28 9:24 ` Peter Meerwald-Stadler
2016-06-28 13:39 ` Quentin Schulz
2016-06-28 14:18 ` Peter Meerwald-Stadler
2016-06-28 16:25 ` Jonathan Cameron
2016-07-03 11:54 ` Jonathan Cameron
2016-07-03 11:54 ` Jonathan Cameron
2016-07-03 12:48 ` Jonathan Cameron
2016-07-03 12:48 ` Jonathan Cameron
2016-07-03 15:43 ` Guenter Roeck
2016-07-03 15:43 ` Guenter Roeck
2016-07-04 7:26 ` Quentin Schulz
2016-07-04 7:26 ` Quentin Schulz
2016-07-04 16:29 ` Guenter Roeck
2016-07-04 16:29 ` Guenter Roeck
2016-07-05 7:40 ` Quentin Schulz
2016-07-05 7:40 ` Quentin Schulz
2016-06-28 8:18 ` [PATCH 3/3] hwmon: iio_hwmon: defer probe when no channel is found Quentin Schulz
2016-06-28 8:18 ` Quentin Schulz
2016-06-30 3:47 ` [3/3] " Guenter Roeck
2016-06-30 3:47 ` Guenter Roeck
2016-06-30 13:59 ` Jonathan Cameron
2016-06-30 13:59 ` Jonathan Cameron
2016-06-30 14:49 ` Lars-Peter Clausen
2016-06-30 14:49 ` Lars-Peter Clausen
2016-06-30 14:51 ` Guenter Roeck
2016-06-30 14:51 ` Guenter Roeck
2016-07-03 10:47 ` Jonathan Cameron
2016-07-03 10:47 ` Jonathan Cameron
2016-07-03 15:48 ` Guenter Roeck
2016-07-03 15:48 ` Guenter Roeck
2016-06-29 3:28 ` [PATCH 0/3] add support for Allwinner SoCs ADC Chen-Yu Tsai
2016-06-29 3:28 ` Chen-Yu Tsai
2016-07-01 9:45 ` Quentin Schulz
2016-07-01 9:45 ` Quentin Schulz
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=57794DB3.2050506@roeck-us.net \
--to=linux@roeck-us.net \
--cc=antoine.tenart@free-electrons.com \
--cc=jdelvare@suse.com \
--cc=jic23@kernel.org \
--cc=knaack.h@gmx.de \
--cc=lars@metafoo.de \
--cc=lee.jones@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-hwmon@vger.kernel.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maxime.ripard@free-electrons.com \
--cc=pmeerw@pmeerw.net \
--cc=quentin.schulz@free-electrons.com \
--cc=thomas.petazzoni@free-electrons.com \
--cc=wens@csie.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 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.