Re: [PATCH v2 0/2] Improve drm_of_component_probe() and move rockchip to use it

[Russell King - ARM Linux <linux@arm.linux.org.uk>]

On Thu, Dec 24, 2015 at 01:27:08PM +0100, Jean-Francois Moine wrote:
> On Thu, 24 Dec 2015 10:52:07 +0000
> Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> > However, when we come to the Linux implementation, things get sticky
> > because we need to select the correct platform device corresponding
> > with the IPU's port.  This can only be done using the 'port' node
> > and not port->parent.
> > 
> > port->parent would be the IPU device node itself.  If we were to
> > introduce the additional ports {} node, that doesn't help, because
> > now port->parent points at the ports {} node instead, not the actual
> > port - and we need the port itself to identify which of the IPU's
> > own created platform devices to select.
> > 
> > So, modifying DT doesn't help in any way, even if you ignore the fact
> > that we need to maintain backwards compatibility.
> 
> The ports {} node is just a container, and so is the (unique) port {}
> node which is inside:
> 
> 	ipu1: ipu@02400000 {
> 		...
> 		ports@2 {			/* di0 device */
> 			ipu1_di0: port {
> 				...
> 				ipu1_di0_hdmi: endpoint@1 {
> 					remote-endpoint = <&hdmi_mux_0>;
> 				};
> 				ipu1_di0_mipi: endpoint@2 {
> 					remote-endpoint = <&mipi_mux_0>;
> 				};
> 				...
> 			};
> 		};
> 		ports@3 {			/* di1 device */
> 			ipu1_di1: port {
> 				...
> 				ipu1_di1_hdmi: endpoint@1 {  
> 					remote-endpoint = <&hdmi_mux_1>;
> 				};
> 				ipu1_di1_mipi: endpoint@2 {
> 					remote-endpoint = <&mipi_mux_1>;
> 				};
> 				...
> 			};
> 		};
> 	};

That's against the binding documentation for graphs:

All 'port' nodes can be grouped under an optional 'ports' node, which
allows to specify #address-cells, #size-cells properties for the 'port'
nodes independently from any other child device nodes a device might
have.

It says "All 'port' nodes" not "Some" or similar.  The DT code requires
this.  To change this would mean changing the DT binding and the code
parsing that binding, adding much more complexity there.

You earlier argued against adding (what would be less) complexity to
the DRM OF helper, now you seem to be wanting more complexity elsewhere
to save what would be trivial complexity elsewhere - all the functions
which iterate over the port nodes would need to be updated to find
all the "ports" nodes, and end up needing an additional level of looping
and complexity to jump from one port node in a ports { } block to the
first port node in the next ports { } block.

Also it makes the API more difficult because we end up with the ports@n
nodes needing a reg= property (as per ePAPR requirements) and it becomes
unclear what that would represent at the hardware level.

It seems that you're trying to work around a limitation in Linux by
modifying the hardware representation...

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.