On Mon, Jan 06, 2014 at 03:52:01PM +0100, Philipp Zabel wrote: > @@ -438,24 +453,21 @@ int imx_drm_encoder_parse_of(struct drm_device *drm, > struct drm_encoder *encoder, struct device_node *np) > { > struct imx_drm_device *imxdrm = drm->dev_private; > + struct device_node *ep, *last_ep = NULL; > uint32_t crtc_mask = 0; > int i, ret = 0; > > for (i = 0; !ret; i++) { > - struct of_phandle_args args; > uint32_t mask; > - int id; > > - ret = of_parse_phandle_with_args(np, "crtcs", "#crtc-cells", i, > - &args); > - if (ret == -ENOENT) > + ep = v4l2_of_get_next_endpoint(np, last_ep); > + if (last_ep) > + of_node_put(last_ep); > + if (!ep) > break; > - if (ret < 0) > - return ret; > > - id = args.args_count > 0 ? args.args[0] : 0; > - mask = imx_drm_find_crtc_mask(imxdrm, args.np, id); > - of_node_put(args.np); > + /* CSI */ > + mask = imx_drm_find_crtc_mask(imxdrm, ep); > > /* > * If we failed to find the CRTC(s) which this encoder is > @@ -463,12 +475,20 @@ int imx_drm_encoder_parse_of(struct drm_device *drm, > * not been registered yet. Defer probing, and hope that > * the required CRTC is added later. > */ > - if (mask == 0) > + if (mask == 0) { > + of_node_put(ep); > return -EPROBE_DEFER; > + } > > crtc_mask |= mask; > + last_ep = ep; > } > > + if (ep) > + of_node_put(ep); Why is this loop soo complicated? Why do you need to mess around with this "last_ep" stuff - you don't actually end up using it. The loop reduces down to this without comments: for (i = 0; !ret; i++) { uint32_t mask; ep = v4l2_of_get_next_endpoint(np, last_ep); if (!ep) break; /* CSI */ mask = imx_drm_find_crtc_mask(imxdrm, ep); of_node_put(ep); if (mask == 0) return -EPROBE_DEFER; crtc_mask |= mask; } Now, here's the big question: why do we want to use v4l2_* here? We may want to use this functionality, but if this functionality is going to be used outside of v4l2, it needs to become something generic, not v4l2 specific. Let's think about this for a moment... if we want to build imx-drm into the kernel, can we do it with modular videodev, or with videodev completely unconfigured. We may wish to do this because we have no videodev requirement on a platform. Should the build fail because the v4l2 function isn't there? So, before we can change this, I think we first need to get agreement from Mauro to move this function out of V4L2, so that it can be available independently of V4L2. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit". _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel