linux-kernel - Re: [PATCH 2/4] dt-bindings: Document the Raspberry Pi Touchscreen nodes.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 19 May 2017 12:32:27 +0300
From:   Laurent Pinchart <laurent.pinchart@...asonboard.com>
To:     Archit Taneja <architt@...eaurora.org>
Cc:     Eric Anholt <eric@...olt.net>, Rob Herring <robh+dt@...nel.org>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        Thierry Reding <thierry.reding@...il.com>,
        Mark Rutland <mark.rutland@....com>,
        Andrzej Hajda <a.hajda@...sung.com>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/4] dt-bindings: Document the Raspberry Pi Touchscreen nodes.

Hi Archit,

On Friday 19 May 2017 14:24:36 Archit Taneja wrote:
> On 05/18/2017 08:25 PM, Laurent Pinchart wrote:
> > On Thursday 18 May 2017 13:56:19 Archit Taneja wrote:
> >> On 05/17/2017 12:16 AM, Eric Anholt wrote:
> >
> > [snip]
> > 
> >>> In terms of physical connections:
> >>>    [15-pin "DSI" connector on 2835]
> >>>     | I2C               | DSI
> >>>    / \        SPI       |
> >>> [TS]  [Atmel]------[TC358762]
> >>>        \                |
> >>>         \PWM            |
> >>>          \              | DPI
> >>> 
> >>> [some backlight]------[some unknown panel]
> >>> 
> >>> The binding I'm trying to create is to expose what's necessary for a
> >>> driver that talks I2C to the Atmel, which then controls the PWM and does
> >>> the command sequence over SPI to the Toshiba that sets up its end of the
> >>> DSI link.
> >> 
> >> The bridge (Atmel + TC358762 combination) here looks like it's primarily
> >> an i2c device (i.e, the control bus is i2c). Therefore, the drm-bridge
> >> driver here should be an i2c driver instead of a mipi_dsi_driver.
> > 
> > Glad to see we agree, that's what I've proposed in a separate answer :-)
> > I'd go one step further though, there should be no DRM bridge, just a DRM
> > panel.
>
> If the PCB containing the controller chips and the panel are part of a
> single casing, and the set up won't work with another panel, then yeah, I
> agree. If the bridge chips are on a separate adapter board, and there is a
> possibility to connect other panels, then maybe a separate DRM bridge and a
> DRM panel might be a safer bet.

I thought it was a single black box, but upon closer inspection there's a 
separate PCB with the Microcontroller and TC358762.

Eric, do you know if it's possible to exchange the panel for another one (and 
not just an model with identical features from another vendor, but another 
panel with a different mode for instance) without reprogramming the 
microcontroller, or is the bridge board tied to the panel model ?

> >> We have the facility to create a mipi DSI device without the need to have
> >> a corresponding node in DT. The ADV7533 and TC358767 drivers are examples
> >> of that.
> >> 
> >> The following is what the binding could look like, it's same as what Rob
> >> also mentioned previously in the thread.
> >> 
> >> Thanks,
> >> Archit
> >> 
> >> dsi1: dsi@...00000 {
> >> 
> >> 	#address-cells = <1>;
> >> 	#size-cells = <0>;
> >> 	<...>
> >> 	
> >> 	/* The SoC's DSI input/output port */
> >> 	ports {
> >> 		#address-cells = <1>;
> >> 		#size-cells = <0>;
> >> 		
> >> 		/* port@0 if needed */
> >> 		port@1 {
> >> 			dsi_out_port: endpoint {
> >> 				reg = <1>;
> >> 				remote-endpoint = <&bridge_dsi_port>;
> >> 			};
> >> 		};
> >> 	};
> >> };
> >> 
> >> i2c_dsi: i2c {
> >> 	compatible = "i2c-gpio";
> >> 	#address-cells = <1>;
> >> 	#size-cells = <0>;
> >> 	gpios = <&gpio 28 0
> >> 		 &gpio 29 0>;
> >> 	
> >> 	/* the Atmel + TC35872 bridge */
> >> 	pitouchscreen_bridge: bridge@45 {
> > 
> > This should thus be lcd@45.
> > 
> >> 		compatible = "raspberrypi,touchscreen-bridge";
> > 
> > And this raspberrypi,7inch-touchscreen-panel. Shame we haven't
> > standardized
> > the vendor name prefix to rpi :-/
> > 
> >> 		reg = <0x45>;
> >> 		
> >> 		ports {
> >> 			#address-cells = <1>;
> >> 			#size-cells = <0>;
> >> 			port@0 {
> >> 				reg = <0>;
> >> 				bridge_dsi_port: endpoint {
> > 
> > This should be named panel_dsi_port.
> > 
> >> 					remote-endpoint = <&dsi_out_port>;
> >> 				};
> >> 			};
> >> 			port@1 {
> >> 				reg = <1>;
> >> 				bridge_dpi_port: endpoint {
> >> 					remote-endpoint =
> > <&pitouchscreen_panel_port>;
> >> 				};
> >> 			};
> > 
> > The second port is thus not needed.
> > 
> >> 		};
> > 
> > So we can simplify this to
> > 
> > 		port {
> > 			panel_dsi_port: endpoint {
> > 				remote-endpoint = <&dsi_out_port>;
> > 			};
> > 		};
> > 
> > (no need for a ports node when there's a single port)
> > 
> >> 	};
> >> };
> >> 
> >> lcd {
> >> 	compatible = "raspberrypi,7inch-touchscreen-panel";
> >> 	ports {
> >> 		#address-cells = <1>;
> >> 		#size-cells = <0>;
> >> 		port@0 {
> >> 			reg = <0>;
> >> 			pitouchscreen_panel_port: endpoint {
> >> 				remote-endpoint = <&bridge_dpi_port>;
> >> 			};
> >> 		};
> >> 	};
> >> };
> > 
> > And this node can go away.

-- 
Regards,

Laurent Pinchart