linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sebastian Reichel <sre@debian.org>
To: "Pali Rohár" <pali.rohar@gmail.com>
Cc: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>,
	David Woodhouse <dwmw2@infradead.org>,
	Jonathan Cameron <jic23@kernel.org>,
	Marek Belisko <marek@goldelico.com>,
	Rob Herring <robh+dt@kernel.org>, Pawel Moll <pawel.moll@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Kumar Gala <galak@codeaurora.org>,
	Grant Likely <grant.likely@linaro.org>,
	LKML <linux-kernel@vger.kernel.org>,
	devicetree@vger.kernel.org, linux-iio@vger.kernel.org,
	Ivajlo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Subject: Re: [PATCHv1 0/2] Convert rx51-battery to IIO API and add DT support
Date: Wed, 26 Feb 2014 18:51:29 +0100	[thread overview]
Message-ID: <20140226175128.GA13975@earth.universe> (raw)
In-Reply-To: <CAHYPw2EeEnDn2FUYSMWbywprmhJmcRRyaTPn1YZK0xPvuiJQFw@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2803 bytes --]

Hi Pali,

On Wed, Feb 26, 2014 at 08:40:54AM +0100, Pali Rohár wrote:
> I would like to ask other kernel developers what do you think about
> moving ADC channel numbers from rx51_battery.ko driver code to DT.
> Driver rx51_battery.ko is platform specific for Nokia RX-51 (N900) so
> it is usefull only for this one device.
> 
> Before this patch all driver data (look-up tables, adc channel
> numbers, etc...) were in driver code. Now after this patch adc channel
> numbers were moved to DT. What do you think? It is better to have all
> data in one place (driver code) or some in DT and some in driver code?
> 
> For me it does not make sense to move these numbers to DT, because
> driver is rx51 device specific and chaning it in DT does not make
> sense. And I think it is better to have add driver data in one place
> and not in two...
> 
> Sebastian already wrote me that this is normal to have numbers in DT
> and other code in driver. But I think that driver which can be used
> only in one device (so specified only in one DT file) does not need to
> have configuration (via DT or board files).
>
> Or do you think that driver specified only for one device needs to
> have ADC numbers configuration via DT?

I think the problem is, that you think of ADC channel numbers as
configuration data. This means you think of rx51-battery as an
alternative driver for the twl4030-madc.

I think of rx51-battery as its own platform device, which makes use
of the ADC similar to a button making use of a GPIO chip.
For me the ADC channel numbers are not configuration data, but an
inter-device resource reference, like e.g. GPIO references.
This is exactly the data you would put into the device tree.

Now let's take the rx51-audio device as another example for an n900
specific device (not yet in mainline kernel). It does not need ADC
channels, but GPIO lines:

	sound: n900-audio {
		/* ... some more references ... */
		nokia,tvout-selection-gpio = <&gpio2 8 GPIO_ACTIVE_HIGH>; /* 40 */
		nokia,jack-detection-gpio = <&gpio6 17 GPIO_ACTIVE_HIGH>; /* 177 */
		nokia,eci-switch-gpio = <&gpio6 22 GPIO_ACTIVE_HIGH>; /* 182 */
		nokia,speaker-amplifier-gpio = <&twl_gpio 7 GPIO_ACTIVE_HIGH>;
	};

Since GPIO numbers are not guaranteed to be consistent in DT boot
mode at least the GPIO chip must be referenced. Do you really think
it's a good idea to leave out the exact GPIO number just because
the driver knows it needs the 8th GPIO from the second GPIO chip?
IMHO this is really ugly, since it splits the information, which
GPIO is used into two parts - one living in the DT and one living
in the driver with no advantage at all. So it does make sense to
specify inter-device resources via DT even for platform specific
devices.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

  reply	other threads:[~2014-02-26 17:51 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-26  0:46 [PATCHv1 0/2] Convert rx51-battery to IIO API and add DT support Sebastian Reichel
2014-02-26  0:46 ` [PATCHv1 1/2] rx51_battery: convert to iio consumer Sebastian Reichel
2014-02-26 21:43   ` Belisko Marek
2014-02-26 21:54     ` Sebastian Reichel
2014-02-27 21:34       ` Belisko Marek
2014-02-28  2:05         ` Sebastian Reichel
2014-02-28 20:32           ` Belisko Marek
2014-02-28 20:59             ` Belisko Marek
2014-02-28 21:08             ` Sebastian Reichel
2014-02-28 21:13               ` Belisko Marek
2014-02-28 22:32                 ` Belisko Marek
2014-02-28 23:22                   ` Sebastian Reichel
2014-03-04 21:20                     ` Belisko Marek
2014-03-04 22:02                       ` Sebastian Reichel
2014-03-01 11:17   ` Jonathan Cameron
2014-03-01 11:22     ` Jonathan Cameron
2014-02-26  0:46 ` [PATCHv1 2/2] Documentation: DT: Document rx51-battery binding Sebastian Reichel
2014-02-26  7:40 ` [PATCHv1 0/2] Convert rx51-battery to IIO API and add DT support Pali Rohár
2014-02-26 17:51   ` Sebastian Reichel [this message]
2014-03-01 20:22 ` [PATCHv2 " Sebastian Reichel
2014-03-01 20:22   ` [PATCHv2 1/2] rx51_battery: convert to iio consumer Sebastian Reichel
2014-03-29 11:09     ` Jonathan Cameron
2014-04-20 12:08       ` Pavel Machek
2014-04-23 16:09         ` Sebastian Reichel
2014-06-14  8:32           ` Pavel Machek
2014-06-14 15:47             ` Sebastian Reichel
2014-03-01 20:22   ` [PATCHv2 2/2] Documentation: DT: Document rx51-battery binding Sebastian Reichel
2014-03-29 11:10     ` Jonathan Cameron
2014-04-20 12:09     ` Pavel Machek

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=20140226175128.GA13975@earth.universe \
    --to=sre@debian.org \
    --cc=dbaryshkov@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dwmw2@infradead.org \
    --cc=galak@codeaurora.org \
    --cc=grant.likely@linaro.org \
    --cc=ijc+devicetree@hellion.org.uk \
    --cc=ivo.g.dimitrov.75@gmail.com \
    --cc=jic23@kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marek@goldelico.com \
    --cc=mark.rutland@arm.com \
    --cc=pali.rohar@gmail.com \
    --cc=pawel.moll@arm.com \
    --cc=robh+dt@kernel.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).