Re: [PATCH] v4l: async: make v4l2 coexists with devicetree nodes in a dt overlay

[Javier Martinez Canillas]

Hello Javi,

On 11/23/2016 07:09 AM, Javi Merino wrote:
> In asd's configured with V4L2_ASYNC_MATCH_OF, if the v4l2 subdev is in
> a devicetree overlay, its of_node pointer will be different each time
> the overlay is applied.  We are not interested in matching the
> pointer, what we want to match is that the path is the one we are
> expecting.  Change to use of_node_cmp() so that we continue matching
> after the overlay has been removed and reapplied.
>

I'm still not that familiar with DT overlays (and I guess others aren't
either) so I think that including an example of a base tree and overlay
DTS where this is an issue, could make things more clear in the commit.

IIUC, it should be something like this?

-- base tree --

&i2c1 {
	camera: camera@10 {
		reg = <0x10>;
		port {
			cam_ep: endpoint {
				...
			};
		};
	};
};

&media_bridge {
	...
	ports {
		port@0 {
			reg = <0>;
			ep: endpoint {
				remote-endpoint = <&cam_ep>;
			};
		};
	};
};

-- overlay --

/plugin/;
/ {
	...
	fragment@0 {
		target = <&camera>;
		__overlay__ {
			compatible = "foo,bar";
			...
			port {
				cam_ep: endpoint {
					...
				};
			};
		};
	}
}

> Cc: Mauro Carvalho Chehab <mchehab@xxxxxxxxxx>
> Cc: Javier Martinez Canillas <javier@xxxxxxxxxxxxxxx>
> Cc: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> Signed-off-by: Javi Merino <javi.merino@xxxxxxxxxx>
> ---
> Hi,
> 
> I feel it is a bit of a hack, but I couldn't think of anything better.
> I'm ccing devicetree@ and Pantelis because there may be a simpler
> solution.
>

I also couldn't think a better way to do this, since IIUC the node's name is
the only thing that doesn't change, and is available at the time the bridge
driver calls v4l2_async_notifier_register() when parsing the base tree.

>  drivers/media/v4l2-core/v4l2-async.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> index 5bada20..d33a17c 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -42,7 +42,8 @@ static bool match_devname(struct v4l2_subdev *sd,
>  
>  static bool match_of(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
>  {
> -	return sd->of_node == asd->match.of.node;
> +	return !of_node_cmp(of_node_full_name(sd->of_node),
> +			    of_node_full_name(asd->match.of.node));
>  }
>  
>  static bool match_custom(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
> 

Reviewed-by: Javier Martinez Canillas <javier@xxxxxxxxxxxxxxx>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America