Re: [RFC PATCH v3 1/4] drm: Introduce generic probe function for component based masters.

On Mon, Oct 19, 2015 at 04:07:47PM +0100, Liviu Dudau wrote:

A lot of component based DRM drivers use a variant of the same code as the probe function. They bind the crtc ports in the first iteration and then scan through the child nodes and bind the encoders attached to the remote endpoints. Factor the common code into a separate function called drm_of_component_probe() in order to increase code reuse.

Sorry, I take back my Acked-by.

• /*

• * Bind the crtc's ports first, so that drm_of_find_possible_crtcs()
  

• * called from encoder's .bind callbacks works as expected
  

• */
  

• for (i = 0; ; i++) {

• port = of_parse_phandle(dev->of_node, "ports", i);
  

• if (!port)
  

• 	break;
  

• if (!of_device_is_available(port->parent)) {
  

• 	of_node_put(port);
  

• 	continue;
  

• }
  

• component_match_add(dev, &match, compare_of, port);
  

• of_node_put(port);
  

We shouldn't be putting the ports here - we're still retaining a reference against the port, even though it's by address. Yes, we have no way to release this reference, which probably ought to be fixed. Please replace this of_node_put() here with a comment to that effect.

• for_each_child_of_node(port, ep) {
  

• 	remote = of_graph_get_remote_port_parent(ep);
  

• 	if (!remote || !of_device_is_available(remote)) {
  

• 		of_node_put(remote);
  

• 		continue;
  

• 	} else if (!of_device_is_available(remote->parent)) {
  

• 		dev_warn(dev, "parent device of %s is not available\n",
  

• 			 remote->full_name);
  

• 		of_node_put(remote);
  

• 		continue;
  

• 	}
  

• 	component_match_add(dev, &match, compare_of, remote);
  

• 	of_node_put(remote);
  

Same here.