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

From: Javier Martinez Canillas
Date: Wed Nov 23 2016 - 09:25:59 EST


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