Hi Laurent, On Fri, Apr 07, 2017 at 01:07:48PM +0300, Laurent Pinchart wrote: > Hi Sakari, > > Thank you for the patch. > > On Thursday 06 Apr 2017 16:12:06 Sakari Ailus wrote: > > OF and fwnode support are separated in V4L2 and individual drivers may > > implement one of them. Sub-devices do not match with a notifier > > expecting sub-devices with fwnodes, nor the other way around. > > Shouldn't we instead convert all drivers to fwnode matching ? What's missing > after the mass conversion in patch 5/8 ? A lot of drivers use the OF frame work and thus do not deal with fwnodes directly. I haven't entirely converted them to use the fwnode API since making additional, unnecessary changes increases the likelihood of errors. > > > Fix this by checking for sub-device's of_node field in fwnode match and > > fwnode field in OF match. > > > > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> > > --- > > drivers/media/v4l2-core/v4l2-async.c | 26 +++++++++++++++++++++++--- > > include/media/v4l2-async.h | 2 +- > > 2 files changed, 24 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/media/v4l2-core/v4l2-async.c > > b/drivers/media/v4l2-core/v4l2-async.c index 384ad5e..7f5d804 100644 > > --- a/drivers/media/v4l2-core/v4l2-async.c > > +++ b/drivers/media/v4l2-core/v4l2-async.c > > @@ -14,6 +14,7 @@ > > #include <linux/list.h> > > #include <linux/module.h> > > #include <linux/mutex.h> > > +#include <linux/of.h> > > #include <linux/platform_device.h> > > #include <linux/slab.h> > > #include <linux/types.h> > > @@ -40,15 +41,34 @@ static bool match_devname(struct v4l2_subdev *sd, > > return !strcmp(asd->match.device_name.name, dev_name(sd->dev)); > > } > > > > +static bool fwnode_cmp(struct fwnode_handle *one, > > + struct fwnode_handle *theother) > > +{ > > + if (!one || !theother) > > + return false; > > + > > + if (one->type != theother->type) > > + return false; > > + > > + if (is_of_node(one)) > > + return !of_node_cmp(of_node_full_name(to_of_node(one)), > > + of_node_full_name(to_of_node(theother))); > > + else > > + return one == theother; > > +} > > + > > static bool match_of(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd) > > { > > - return !of_node_cmp(of_node_full_name(sd->of_node), > > - of_node_full_name(asd->match.of.node)); > > + return fwnode_cmp(sd->of_node ? > > + of_fwnode_handle(sd->of_node) : sd->fwnode, > > + of_fwnode_handle(asd->match.of.node)); > > } > > > > static bool match_fwnode(struct v4l2_subdev *sd, struct v4l2_async_subdev > > *asd) > > { > > - return sd->fwnode == asd->match.fwnode.fwn; > > + return fwnode_cmp(sd->of_node ? > > + of_fwnode_handle(sd->of_node) : sd->fwnode, > > + asd->match.fwnode.fwn); > > } > > > > static bool match_custom(struct v4l2_subdev *sd, struct v4l2_async_subdev > > *asd) diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h > > index 8f552d2..df8b682 100644 > > --- a/include/media/v4l2-async.h > > +++ b/include/media/v4l2-async.h > > @@ -57,7 +57,7 @@ struct v4l2_async_subdev { > > enum v4l2_async_match_type match_type; > > union { > > struct { > > - const struct device_node *node; > > + struct device_node *node; > > That seems to be a bit of a hack :-( I'd rather make everything const and cast > to non-const pointers explicitly where the API requires us to. Or, better, add > a to_of_node_const() function. I'll see what I can do to the matter, but if you don't mind, I'll base it on this patchset. -- Kind regards, Sakari Ailus e-mail: sakari.ailus@xxxxxx XMPP: sailus@xxxxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html