[PATCH v1 3/3] ARM: dts: am335x-bone: add support for beaglebone LCD4 cape

[Gupta, Pekon]

>From: Jason Kridner [mailto:jkridner at gmail.com]
>On Tue, Jun 24, 2014 at 8:24 AM, Pekon Gupta <pekon at ti.com> wrote:
>> This patch adds support for LCD4 cape as advertised on
>>   http://elinux.org/CircuitCo:BeagleBone_LCD4
[...]

>>
>> diff --git a/arch/arm/boot/dts/am335x-bone-display-cape.dts b/arch/arm/boot/dts/am335x-bone-
>display-cape.dts
>
>If this is mean to be included, why not use the .dtsi extension rather
>than .dts. .dts implies it is a top-level file.
>
I followed the ideology given in in below discussion
http://comments.gmane.org/gmane.linux.drivers.devicetree/13921

As I understood .dtsi file extension should be used for silicon DT data which
which remains common for all the boards using that silicon (like am33xx.dtsi).
And .dts is for board specific DT data.
However, anything works for me, as its just matter of file-name-extension.	
So should I rename all files to .dtsi ?

[...]

>> +/ {
>> +       backlight {
>> +               status = "disabled";
>> +               compatible = "gpio-backlight";
>> +               pinctrl-names = "default";
>> +               pinctrl-0 = <&bbcape_backlight_pins>;
>> +               gpios = <&gpio1 18 GPIO_ACTIVE_HIGH>;
>> +               default-on;
>> +       };
>> +
>> +       panel {
>> +               status = "disabled";
>
>Where are you expecting to enable this? I'm guessing through your
>u-boot hacking? Certainly I can see editing the boot script to make
>this work, but I don't see how this results in the average user simply
>plugging in the cape and having it work without having to edit files.
>
Yes, using u-boot commands, *without touching any source file*.
I'm planning to send patch for adding following helper commands  in u-boot
u-boot> fdt node enable <node-path>
u-boot> fdt node disable <node-path>
If that's accepted, enabling disabling capes will become very easy.
And, as you said user can put these commands in scripts (like Uenv.txt)
or environment variable, and execute them automatically at each boot.


>Perhaps you can put some real logic into the bootloader that looks at
>the cape EEPROMs?
>
No, I don't want to go the EEPROM way, because of reasons suggested
in earlier mail. It's not scalable, especially when you don't have control
over what type and how third-party is developing the capes.


>Have you considered using tools like pinmux-helper as is done with the
>cape-universal overlay?
>https://github.com/beagleboard/devicetree-source/blob/master/arch/arm/boot/dts/cape-universal-
>00A0.dts
>
Yes, I have seen that. In pin-mux helper pins are configured not based on
their actual names but as they appear on P8 and P9 headers of Beaglebone.
- For shared pins, you have defined all possible functionality.
- For dedicated pins, there is single fragment.
Now there are two questions..
(1) Do we need to update this cape-universal-xx.dts everytime a new
Cape in market uses some dedicated pin for some muxed functionality ?
(2) I'm not sure but kernel does _not_ free the memory allocated to 
DT fragment. If so, this universal fragment which has pin-mux for all
P8 and P9 pins will block the memory forever. Would be good to know
the in-memory size required for this universal fragment ?


>I've been hacking with adding all of these pinmux helpers to the main
>.dts. I think you are encouraging me to add it to include files to
>make it a bit more granular.
>
>http://paste.debian.net/106319/
>
Yes, using include files would be good.
Also one minor feedback. Please use macros for Pin directions, Pulls, etc
instead of hex numbers, it make it more readable. And then you don't need
to specify that in comments.
-  0xa0 0x08	/* lcd_data0.lcd_data0, OMAP_MUX_MODE0 | AM33XX_PIN_OUTPUT | AM33XX_PULL_DISA */
+ 0xa0	(OMAP_MUX_MODE0 | AM33XX_PIN_OUTPUT | AM33XX_PULL_DISA)    /* lcd_data0.lcd_data0 */


>I appreciate the enthusiasm to get this support into the mainline. If
>it goes, I'll try to follow with a bunch of other outstanding changes
>we have sitting in the vendor tree right now.
>
Thanks. Waiting for Tony to at-least 'Ack' or say if this is acceptable approach.


>Have you looked at the features in the devicetree for this cape that
>is currently pushed in the vendor tree?
>
>https://github.com/beagleboard/devicetree-source/blob/master/arch/arm/boot/dts/BB-BONE-LCD4-
>01-00A1.dts
>
Yes, I have not added touchscreen support.
I developed this DTS independently while debugging some Display v/s NAND issue.
If these DTS patches are acceptable, then I'll refine them further.


with regards, pekon