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

Laurent Pinchart
2017-05-18T14:55:26Z

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.

> 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.

--
Regards,

Laurent Pinchart