Devicetree

* Re: [PATCHv1 0/2] Convert rx51-battery to IIO API and add DT support
From: Sebastian Reichel @ 2014-02-26 17:51 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Dmitry Eremin-Solenikov, David Woodhouse, Jonathan Cameron,
	Marek Belisko, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Grant Likely, LKML,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA, Ivajlo Dimitrov
In-Reply-To: <CAHYPw2EeEnDn2FUYSMWbywprmhJmcRRyaTPn1YZK0xPvuiJQFw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

[-- 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 --]

^ permalink raw reply