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
