Re: [PATCH 13/26] ARM: omap3.dtsi: add omapdss information

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

[-- Attachment #1: Type: text/plain, Size: 3465 bytes --]

Hi Tomi,

On Monday 16 December 2013 12:49:03 Tomi Valkeinen wrote:
> On 2013-12-13 05:24, Laurent Pinchart wrote:
> >> Then DPI, which I think is mostly just level shifters. It's really just
> >> a port, as you say.
> >> 
> >> SDI is a bit unclear to me. The registers for it are in the dss_core.
> >> There's only a few registers, but it does have a PHY and a PLL. But I
> >> guess it's also more of a port than a separate module.
> > 
> > After a quick look at the documentation I would say so. I would be tempted
> > to consider RFBI as part of the DSS core, but that's less clear.
> 
> I had a look at this, mainly the DPI side so far. There's one extra
> complication, which actually affects all other outputs also (and CDF):
> pinctrl.
> 
> In the current series, I just have pinctrl for each device, with
> "default" name, which ends up being used by default without any code on
> my part.
> 
> However, if DPI is no longer a device, it can't have pinctrl entry. But
> this is a wider issue, as the pinctrl should really be per endpoint, not
> per device. When an endpoint is selected to be used, a particular
> pinmuxing should be taken into use.
> 
> I'm not sure what would be the cleanest solution to this. I currently
> have this working:
> 
> &dss {
> 	pinctrl-names = "default-0-0";
> 	pinctrl-0 = <&dss_dpi_pins>;
> 
> 	port@0 {
> 		dpi_out: endpoint {
> 			remote-endpoint = <&tfp410_in>;
> 			data-lines = <24>;
> 		};
> 	};
> };
> 
> So here I have 'port@0' for DSS, which is the DPI output, and it has a
> single endpoint. For DSS device, I have pinctrl data.
> 
> When the DPI endpoint is initialized, the code looks for pinctrl with
> name "default-<portnum>-<endpointnum>". As the DPI is port 0, and just
> one endpoint, the code looks for "default-0-0".
> 
> For omap3 board with both DPI and SDI as options (they can't be used at
> the same time, though), I imagine it'd be like:
> 
> &dss {
> 	vdds_dsi-supply = <&vpll2>;
> 	vdds_sdi-supply = <&vpll2>;
> 
> 	pinctrl-names = "default-0-0", "default-1-0";
> 	pinctrl-0 = <&dss_dpi_pins>;
> 	pinctrl-1 = <&dss_sdi_pins>;
> 
> 	ports {
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		port@0 {
> 			reg = <0>;
> 			dpi_out: endpoint {
> 			};
> 		};
> 
> 		port@1 {
> 			reg = <1>;
> 			sdi_out: endpoint {
> 			};
> 		};
> 	};
> };
> 
> Any thoughts?

Would it be feasible to put the pinctrl properties in the port or endpoint 
nodes ? That could require changes to the pinctrl core, most probably just 
exporting a few internal functions (possibly requiring a bit of refactoring), 
but it might make the result simpler.

> Every time I work with ports/endpoints, I feel that this is needlessly
> complex. But I have never come up with any cleaner or simpler way to
> handle this.
>
> I also think this multiple-peripherals-per-single-port is not really
> display related, although, for some reason, it seems like display is the
> most abused hardware. Maybe ports/endpoints or similar should be in the
> common driver framework?

Ports and endpoints is the way we have come up with to describe a graph in DT. 
I wouldn't call it needlessly complex, as I believe it's both generic and 
simple, but I agree it's a bit on the verbose side. Omitting the ports and 
port nodes as a shortcut might be a good way to reduce the verbosity.

Regarding moving this to the device core, I'm not opposed to it, but I'd like 
to see interest from other users first.

-- 
Regards,

Laurent Pinchart

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]