linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: pza@pengutronix.de (Philipp Zabel)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH 2/3] staging: imx-drm-core: Use graph to find connection between crtc and encoder
Date: Mon, 10 Feb 2014 21:45:45 +0100	[thread overview]
Message-ID: <20140210204545.GA2793@pengutronix.de> (raw)
In-Reply-To: <20140210162631.GC26684@n2100.arm.linux.org.uk>

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

  reply	other threads:[~2014-02-10 20:45 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-06 14:51 [RFC PATCH 0/3] imx-drm dt bindings Philipp Zabel
2014-01-06 14:52 ` [RFC PATCH 1/3] ARM: dts: imx6qdl: Add ports and endpoints to IPU DIs Philipp Zabel
2014-01-06 14:52 ` [RFC PATCH 2/3] staging: imx-drm-core: Use graph to find connection between crtc and encoder Philipp Zabel
2014-02-10 16:26   ` Russell King - ARM Linux
2014-02-10 20:45     ` Philipp Zabel [this message]
2014-02-11  8:38     ` Dan Carpenter
2014-02-11  9:00       ` Philipp Zabel
2014-01-06 14:52 ` [RFC PATCH 3/3] staging: imx-drm-core: associate crtc devices with di port nodes Philipp Zabel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140210204545.GA2793@pengutronix.de \
    --to=pza@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).