Re: [PATCH 2/2] ARM: dts: da850: add a node for the LCD controller

[Sekhar Nori <nsekhar@ti.com>]

On Thursday 20 October 2016 03:51 PM, Tomi Valkeinen wrote:
> On 20/10/16 13:07, Sekhar Nori wrote:
> 
>> Per me, compatible property is an ordered list precisely for the reason
>> that things should continue to "work" with as closely matched driver as
>> possible. So even if someone is running a kernel which does not
>> recognize "ti,da850-tilcdc", it should still be able to probe the driver
>> based on "ti,am33xx-tilcdc" and provide as close to full functionality
>> as possible.
>>
>> That said, I will not insist on keeping it around if Tomi is
>> uncomfortable. And having read the binding documentation accepted by
>> Jyri, it actually says the compatible property should be __one of__
>> "ti,am33xx-tilcdc" or "ti,da850-tilcdc".
> 
> Well, they are just not compatible as far as I know. If the LCDC on
> DA850 would be identified as AM335x LCDC, and used as such, it would not
> work at all. They have different registers, AM335x LCDC has registers
> that do not exist on DA850.
> 
> With our driver it happens to work, because the driver looks at the IP
> revision in the registers, and then decides that this IP is not AM335x
> LCDC even if the dts says so. But I see that as a driver "feature",
> nothing that the .dts can depend on.
> 
> Perhaps it might work the other way around, using DA850 driver on
> AM335x, as DA850 LCDC is a kind of subset of AM335x LCDC. But I'm not
> sure even about that.

Alright, thanks for the detailed explanation. I dropped the "ti,am33xx-
tilcdc" from the list and here is the updated patch I am queuing for
reference.

Thanks,
Sekhar

--8<--
Author:     Karl Beldan <kbeldan@baylibre.com>
AuthorDate: Wed Oct 5 15:05:32 2016 +0200
Commit:     Sekhar Nori <nsekhar@ti.com>
CommitDate: Thu Oct 20 15:57:21 2016 +0530

    ARM: dts: da850: add a node for the LCD controller
    
    Add pins used by the LCD controller and a disabled LCDC node to be
    reused in device trees including da850.dtsi.
    
    Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
    [Bartosz:
      - added the commit description
      - changed the dt node name to a generic one
      - added a da850-specific compatible string
      - removed the tilcdc,panel node
      - moved the pins definitions to da850.dtsi as suggested by
        Sekhar Nori (was in: da850-lcdk.dts)]
    Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
    [nsekhar@ti.com: fix compatible property and remove interrupt-parent]
    Signed-off-by: Sekhar Nori <nsekhar@ti.com>

diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index f79e1b91c680..901d5c98d5f0 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -186,6 +186,27 @@
 					0xc 0x88888888 0xffffffff
 				>;
 			};
+			lcd_pins: pinmux_lcd_pins {
+				pinctrl-single,bits = <
+					/*
+					 * LCD_D[2], LCD_D[3], LCD_D[4], LCD_D[5],
+					 * LCD_D[6], LCD_D[7]
+					 */
+					0x40 0x22222200 0xffffff00
+					/*
+					 * LCD_D[10], LCD_D[11], LCD_D[12], LCD_D[13],
+					 * LCD_D[14], LCD_D[15], LCD_D[0], LCD_D[1]
+					 */
+					0x44 0x22222222 0xffffffff
+					/* LCD_D[8], LCD_D[9] */
+					0x48 0x00000022 0x000000ff
+
+					/* LCD_PCLK */
+					0x48 0x02000000 0x0f000000
+					/* LCD_AC_ENB_CS, LCD_VSYNC, LCD_HSYNC */
+					0x4c 0x02000022 0x0f0000ff
+				>;
+			};
 
 		};
 		edma0: edma@0 {
@@ -399,6 +420,13 @@
 				<&edma0 0 1>;
 			dma-names = "tx", "rx";
 		};
+
+		display: display@213000 {
+			compatible = "ti,da850-tilcdc";
+			reg = <0x213000 0x1000>;
+			interrupts = <52>;
+			status = "disabled";
+		};
 	};
 	aemif: aemif@68000000 {
 		compatible = "ti,da850-aemif";