On Mon, Feb 10, 2014 at 04:26:31PM +0000, Russell King - ARM Linux wrote:
[...]
> 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 last_ep dance is necessary because v4l2_of_get_next_endpoint(node,prev)
does not decrement the reference count of its prev argument.
To make the loop as simple as you propose, I'd either have to change
the get_next_endpoint library function or wrap it to release the prev
node:

static struct device_node *imx_drm_of_get_next_endpoint(
                struct device_node *node, struct device_node *prev)
{
        node = v4l2_of_get_next_endpoint(node, prev);
        of_node_put(prev);
        return node;
}
 
> 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;
>       }

Yes, that is easier to read.

> 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.

Either that, or we add a temporary copy of the v4l2_of_get_next_endpoint
and v4l2_of_get_remote_port functions while doing the same.

regards
Philipp

_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to