Re: [PATCH RESEND v3 1/6] drm/of: Change the prototype of drm_of_lvds_get_dual_link_pixel_order

[Maxime Ripard <maxime@cerno.tech>]

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

Hi Laurent,

On Mon, Oct 12, 2020 at 02:00:30AM +0300, Laurent Pinchart wrote:
> > -static int drm_of_lvds_get_remote_pixels_type(
> > -			const struct device_node *port_node)
> > +static int drm_of_lvds_get_remote_pixels_type(const struct device_node *endpoint)
> >  {
> > -	struct device_node *endpoint = NULL;
> > -	int pixels_type = -EPIPE;
> > +	struct device_node *remote_port;
> > +	int pixels_type;
> >  
> > -	for_each_child_of_node(port_node, endpoint) {
> > -		struct device_node *remote_port;
> > -		int current_pt;
> > -
> > -		if (!of_node_name_eq(endpoint, "endpoint"))
> > -			continue;
> > -
> > -		remote_port = of_graph_get_remote_port(endpoint);
> > -		if (!remote_port) {
> > -			of_node_put(remote_port);
> > -			return -EPIPE;
> > -		}
> > -
> > -		current_pt = drm_of_lvds_get_port_pixels_type(remote_port);
> > +	remote_port = of_graph_get_remote_port(endpoint);
> > +	if (!remote_port) {
> >  		of_node_put(remote_port);
> 
> You can drop this line.
> 
> > -		if (pixels_type < 0)
> > -			pixels_type = current_pt;
> > -
> > -		/*
> > -		 * Sanity check, ensure that all remote endpoints have the same
> > -		 * pixel type. We may lift this restriction later if we need to
> > -		 * support multiple sinks with different dual-link
> > -		 * configurations by passing the endpoints explicitly to
> > -		 * drm_of_lvds_get_dual_link_pixel_order().
> > -		 */
> 
> Shouldn't we keep this check when endpoint_id is -1 in
> drm_of_lvds_get_dual_link_pixel_order() ?

I tried to do that, and I'm not quite really sure how to go around it.

This scans all the endpoints in a given port.

However, now that we have the device, port id and endpoint id, we need
to use of_graph_get_port_by_id to get the port matching the device and
port id, and iterate over all its endpoint as done here.

The trouble is that of_graph_get_port_by_id expects a !const
device_node, yet drm_of_lvds_get_dual_link_pixel_order (and seems to be
doing so rightfully), so that creates a warning because we drop the
const there.

Changing the prototype to passing only the port device_node doesn't
really work either, since it would be const, and we would need to call
of_graph_get_endpoint_by_regs (so having the parent device_node, through
of_graph_get_port_parent) and of_graph_get_port_parent takes a !const
port device_node.

I guess we could drop const entirely from our function, but that doesn't
look right either..

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]