Re: [RFC v2 PATCH v4 09/14] ARM: dts: s6e3fa0: add DT bindings

On 04/21/2014 02:28 PM, YoungJun Cho wrote:

This patch adds DT bindings for s6e3fa0 panel. The bindings describes panel resources, display timings and cpu timings.

Changelog v2:

• Adds unit address (commented by Sachin Kamat)

Changelog v3:

• Removes optional delay, size properties (commented by Laurent Pinchart)
• Adds OLED detection, TE gpio properties

Changelog v4:

• Moves CPU timings relevant properties from FIMD DT (commeted by Laurent Pinchart, Andrzej Hajda)

Signed-off-by: YoungJun Cho yj44.cho@samsung.com Acked-by: Inki Dae inki.dae@samsung.com Acked-by: Kyungmin Park kyungmin.park@samsung.com

---

.../devicetree/bindings/panel/samsung,s6e3fa0.txt | 63 ++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt

diff --git a/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt new file mode 100644 index 0000000..9eeb38b --- /dev/null +++ b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt @@ -0,0 +1,63 @@ +Samsung S6E3FA0 AMOLED LCD 5.7 inch panel

+Required properties:

• • compatible: "samsung,s6e3fa0"
• • reg: the virtual channel number of a DSI peripheral
• • vdd3-supply: core voltage supply
• • vci-supply: voltage supply for analog circuits
• • reset-gpio: a GPIO spec for the reset pin
• • det-gpio: a GPIO spec for the OLED detection pin
• • te-gpio: a GPIO spec for the TE pin
• • display-timings: timings for the connected panel as described by [1]

Which properties of display-timings are relevant for CPU mode? I guess width and height. Anything more?

• • cpu-timings: CPU interface timings for the connected panel, and it contains

•    following optional properties.
  

•      - cs-setup: clock cycles for the active period of address signal
  

•            enable until chip select is enable in CPU interface
  

•      - wr-setup: clock cycles for the active period of CS signal enable
  

•            until write signal is enable in CPU interface
  

•      - wr-act: clock cycles for the active period of CS enable in CPU
  

•            interface
  

•      - wr-hold: clock cycles for the active period of CS disable until
  

•            write signal is disable in CPU interface
  

cpu-timings name does not sound to be related to display. I wonder if it would not be better to merge cpu-timings into display-timings but this will require more discussion I guess.

If you want to stay with separate node please consider to make it optional as whole node or make some properties required. Making node required with all sub-properties optional is weird. By the way I hope all timings properties are generic for CPU mode, if not they should be prefixed by vendor or model.

Regards Andrzej

+Optional properties:

+The device node can contain one 'port' child node with one child +'endpoint' node, according to the bindings defined in [2]. This +node should describe panel's video bus.

+[1]: Documentation/devicetree/bindings/video/display-timing.txt +[2]: Documentation/devicetree/bindings/media/video-interfaces.txt

+Example:

• panel@0 {

• compatible = "samsung,s6e3fa0";
  

• reg = <0>;
  

• vdd3-supply = <&vcclcd_reg>;
  

• vci-supply = <&vlcd_reg>;
  

• reset-gpio = <&gpy7 4 0>;
  

• det-gpio = <&gpg0 6 0>;
  

• te-gpio = <&gpd1 7 0>;
  

• display-timings {
  

• 	timing0: timing-0 {
  

• 		clock-frequency = <0>;
  

• 		hactive = <1080>;
  

• 		vactive = <1920>;
  

• 		hfront-porch = <2>;
  

• 		hback-porch = <2>;
  

• 		hsync-len = <1>;
  

• 		vfront-porch = <1>;
  

• 		vback-porch = <4>;
  

• 		vsync-len = <1>;
  

• 	};
  

• };
  

• cpu-timings {
  

• 	cs-setup = <0>;
  

• 	wr-setup = <0>;
  

• 	wr-act = <1>;
  

• 	wr-hold = <0>;
  

• };
  

• };