[PATCH v2 0/2] v4l: async: Match parent devices

[PATCH v2 0/2] v4l: async: Match parent devices

[Kieran Bingham]

From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

As devices become more complicated, it becomes necessary (and more
accurate) to match devices based on their endpoint, as devices may
have multiple subdevices.

To support using endpoints in the V4L2 async subdev framework, while
some devices still use their device fwnode, we need to be able to parse
a fwnode for the device from the endpoint.

By providing a helper fwnode_graph_get_port_parent(), we can use it in
the match_fwnode to support matches during the transition to endpoint
matching.

This series is dependant upon Sakari's v4l2-acpi and acpi-graph-cleaned
branch

v2:
 - Rebased on top of git.linuxtv.org/sailus/media_tree.git #acpi-graph-cleaned

Kieran Bingham (2):
  device property: Add fwnode_graph_get_port_parent
  v4l: async: Match parent devices

 drivers/base/property.c              | 13 +++++++++++-
 drivers/media/v4l2-core/v4l2-async.c | 33 ++++++++++++++++++++++++-----
 include/linux/property.h             |  2 ++-
 3 files changed, 43 insertions(+), 5 deletions(-)

base-commit: d043978c7c919c727fb76b6593c71d0e697a5d66
-- 
git-series 0.9.1

[PATCH v2 1/2] device property: Add fwnode_graph_get_port_parent

[Kieran Bingham]

From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

Provide a helper to obtain the parent device fwnode without first
parsing the remote-endpoint as per fwnode_graph_get_remote_port_parent.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

---
v2:
 - Rebase on top of Sakari's acpi-graph-clean branch and simplify

 drivers/base/property.c  | 13 +++++++++++++
 include/linux/property.h |  2 ++
 2 files changed, 15 insertions(+)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index b311a6fa7d0c..310c53b6b7fc 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1169,6 +1169,19 @@ fwnode_graph_get_next_endpoint(struct fwnode_handle *fwnode,
 EXPORT_SYMBOL_GPL(fwnode_graph_get_next_endpoint);
 
 /**
+ * fwnode_graph_get_port_parent - Return the device fwnode of a port endpoint
+ * @endpoint: Endpoint firmware node of the port
+ *
+ * Returns firmware node of the device the @endpoint belongs to.
+ */
+struct fwnode_handle *
+fwnode_graph_get_port_parent(struct fwnode_handle *endpoint)
+{
+	return fwnode_call_ptr_op(endpoint, graph_get_port_parent);
+}
+EXPORT_SYMBOL_GPL(fwnode_graph_get_port_parent);
+
+/**
  * fwnode_graph_get_remote_port_parent - Return fwnode of a remote device
  * @fwnode: Endpoint firmware node pointing to the remote endpoint
  *
diff --git a/include/linux/property.h b/include/linux/property.h
index b9f4838d9882..af95d5d84192 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -275,6 +275,8 @@ void *device_get_mac_address(struct device *dev, char *addr, int alen);
 
 struct fwnode_handle *fwnode_graph_get_next_endpoint(
 	struct fwnode_handle *fwnode, struct fwnode_handle *prev);
+struct fwnode_handle *fwnode_graph_get_port_parent(
+	struct fwnode_handle *fwnode);
 struct fwnode_handle *fwnode_graph_get_remote_port_parent(
 	struct fwnode_handle *fwnode);
 struct fwnode_handle *fwnode_graph_get_remote_port(
-- 
git-series 0.9.1

[PATCH v2 2/2] v4l: async: Match parent devices

[Kieran Bingham]

From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

Devices supporting multiple endpoints on a single device node must set
their subdevice fwnode to the endpoint to allow distinct comparisons.

Adapt the match_fwnode call to compare against the provided fwnodes
first, but also to search for a comparison against the parent fwnode.

This allows notifiers to pass the endpoint for comparison and still
support existing subdevices which store their default parent device
node.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

---
v2:
 - Added documentation comments
 - simplified the OF match by adding match_fwnode_of()

 drivers/media/v4l2-core/v4l2-async.c | 33 ++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index cbd919d4edd2..2473c0a1f7a8 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -41,14 +41,37 @@ static bool match_devname(struct v4l2_subdev *sd,
 	return !strcmp(asd->match.device_name.name, dev_name(sd->dev));
 }
 
+static bool match_fwnode_of(struct fwnode_handle *a, struct fwnode_handle *b)
+{
+	return !of_node_cmp(of_node_full_name(to_of_node(a)),
+			    of_node_full_name(to_of_node(b)));
+}
+
+/*
+ * Compare the sd with the notifier.
+ *
+ * As a measure to support drivers which have not been converted to use
+ * endpoint matching, we also find the parent device of the node in the
+ * notifier, and compare the sd against that device.
+ */
 static bool match_fwnode(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
 {
-	if (!is_of_node(sd->fwnode) || !is_of_node(asd->match.fwnode.fwnode))
-		return sd->fwnode == asd->match.fwnode.fwnode;
+	struct fwnode_handle *asd_fwnode = asd->match.fwnode.fwnode;
+	struct fwnode_handle *sd_parent, *asd_parent;
+
+	asd_parent = fwnode_graph_get_port_parent(asd_fwnode);
+
+	if (!is_of_node(sd->fwnode) || !is_of_node(asd_fwnode))
+		return sd->fwnode == asd_fwnode ||
+		       sd_parent == asd_fwnode ||
+		       sd->fwnode == asd_parent;
 
-	return !of_node_cmp(of_node_full_name(to_of_node(sd->fwnode)),
-			    of_node_full_name(
-				    to_of_node(asd->match.fwnode.fwnode)));
+	/*
+	 * Compare OF nodes with a full match to support removable dt snippets.
+	 */
+	return match_fwnode_of(sd->fwnode, asd_fwnode) ||
+	       match_fwnode_of(sd_parent, asd_fwnode) ||
+	       match_fwnode_of(sd->fwnode, asd_parent);
 }
 
 static bool match_custom(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
-- 
git-series 0.9.1

Re: [PATCH v2 1/2] device property: Add fwnode_graph_get_port_parent

[Sakari Ailus]

Hi Kieran,

On Fri, May 19, 2017 at 05:16:02PM +0100, Kieran Bingham wrote:
> +struct fwnode_handle *
> +fwnode_graph_get_port_parent(struct fwnode_handle *endpoint)
> +{
> +	return fwnode_call_ptr_op(endpoint, graph_get_port_parent);

graph_get_port_parent op will actually get the parent of the port. But it
expects a port node, not an endpoint node. This is implemented so in order
to center the ops around primitives rather than end user APIs that may
change over time.

I think you'll need:

	return fwnode_call_ptr_op(fwnode_graph_get_next_parent(endpoint),
				  graph_get_port_parent);

Or something like that.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [PATCH v2 1/2] device property: Add fwnode_graph_get_port_parent

[Kieran Bingham]

Hi Sakari,

On 19/05/17 22:51, Sakari Ailus wrote:
> Hi Kieran,
> 
> On Fri, May 19, 2017 at 05:16:02PM +0100, Kieran Bingham wrote:
>> +struct fwnode_handle *
>> +fwnode_graph_get_port_parent(struct fwnode_handle *endpoint)
>> +{
>> +	return fwnode_call_ptr_op(endpoint, graph_get_port_parent);
> 
> graph_get_port_parent op will actually get the parent of the port. But it
> expects a port node, not an endpoint node. This is implemented so in order
> to center the ops around primitives rather than end user APIs that may
> change over time.
> 
> I think you'll need:
> 
> 	return fwnode_call_ptr_op(fwnode_graph_get_next_parent(endpoint),
> 				  graph_get_port_parent);
> 
> Or something like that.

Aha - that explains why I remember thinking to ask you if the implementation of
graph_get_port_parent checked enough levels up :)

I've added the fwnode_graph_get_next_parent() call, but separated it out so that
the code fits cleanly:

struct fwnode_handle *
fwnode_graph_get_port_parent(struct fwnode_handle *endpoint)
{
	struct fwnode_handle *port = fwnode_get_next_parent(endpoint);

	return fwnode_call_ptr_op(port, graph_get_port_parent);
}
EXPORT_SYMBOL_GPL(fwnode_graph_get_port_parent);

I will include this in my testing and rebasing before I repost.
--
Regards

Kieran

Re: [PATCH v2 2/2] v4l: async: Match parent devices

[Kieran Bingham]

Reviewing my own post:

On 19/05/17 17:16, Kieran Bingham wrote:
> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> Devices supporting multiple endpoints on a single device node must set
> their subdevice fwnode to the endpoint to allow distinct comparisons.
> 
> Adapt the match_fwnode call to compare against the provided fwnodes
> first, but also to search for a comparison against the parent fwnode.
> 
> This allows notifiers to pass the endpoint for comparison and still
> support existing subdevices which store their default parent device
> node.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> ---
> v2:
>  - Added documentation comments
>  - simplified the OF match by adding match_fwnode_of()
> 
>  drivers/media/v4l2-core/v4l2-async.c | 33 ++++++++++++++++++++++++-----
>  1 file changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> index cbd919d4edd2..2473c0a1f7a8 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -41,14 +41,37 @@ static bool match_devname(struct v4l2_subdev *sd,
>  	return !strcmp(asd->match.device_name.name, dev_name(sd->dev));
>  }
>  
> +static bool match_fwnode_of(struct fwnode_handle *a, struct fwnode_handle *b)
> +{
> +	return !of_node_cmp(of_node_full_name(to_of_node(a)),
> +			    of_node_full_name(to_of_node(b)));
> +}
> +
> +/*
> + * Compare the sd with the notifier.
> + *
> + * As a measure to support drivers which have not been converted to use
> + * endpoint matching, we also find the parent device of the node in the
> + * notifier, and compare the sd against that device.
> + */
>  static bool match_fwnode(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
>  {
> -	if (!is_of_node(sd->fwnode) || !is_of_node(asd->match.fwnode.fwnode))
> -		return sd->fwnode == asd->match.fwnode.fwnode;
> +	struct fwnode_handle *asd_fwnode = asd->match.fwnode.fwnode;
> +	struct fwnode_handle *sd_parent, *asd_parent;
> +

The keen eyed will notice that sd_parent is not initialised here before use:
Fixed in the next version, pending testing and repost.

> +	asd_parent = fwnode_graph_get_port_parent(asd_fwnode);
> +
> +	if (!is_of_node(sd->fwnode) || !is_of_node(asd_fwnode))
> +		return sd->fwnode == asd_fwnode ||
> +		       sd_parent == asd_fwnode ||
> +		       sd->fwnode == asd_parent;
>  
> -	return !of_node_cmp(of_node_full_name(to_of_node(sd->fwnode)),
> -			    of_node_full_name(
> -				    to_of_node(asd->match.fwnode.fwnode)));
> +	/*
> +	 * Compare OF nodes with a full match to support removable dt snippets.
> +	 */
> +	return match_fwnode_of(sd->fwnode, asd_fwnode) ||
> +	       match_fwnode_of(sd_parent, asd_fwnode) ||
> +	       match_fwnode_of(sd->fwnode, asd_parent);
>  }
>  
>  static bool match_custom(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
>