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

From: Russell King - ARM Linux
Date: Mon Nov 09 2015 - 06:16:07 EST


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.

--
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/