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

[Archit Taneja]

On 05/18/2017 08:25 PM, Laurent Pinchart wrote:
> Hi Archit,
>
> 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.

Thanks,
Archit

> > 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@7e700000 {
> > 	#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.

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project