Hi Jacopo,
On Wed, Mar 18, 2020 at 03:03:26PM +0100, Jacopo Mondi wrote:
> On Wed, Mar 18, 2020 at 02:25:06AM +0200, Laurent Pinchart wrote:
> > When a notifier supplies a device fwnode and a subdev supplies an
> > endpoint fwnode, incorrect matches may occur if multiple subdevs
> > correspond to the same device fwnode. This can't be handled
> > transparently in the framework, and requires the notifier to switch to
> > endpoint fwnodes. Log a message to notify of this problem. A second
> > message is added to help accelerating the transition to endpoint
> > matching.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> > ---
> > drivers/media/v4l2-core/v4l2-async.c | 24 +++++++++++++++++++++++-
> > 1 file changed, 23 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> > index 224b39a7aeb1..9f393a7be455 100644
> > --- a/drivers/media/v4l2-core/v4l2-async.c
> > +++ b/drivers/media/v4l2-core/v4l2-async.c
> > @@ -77,6 +77,7 @@ static bool match_fwnode(struct v4l2_async_notifier *notifier,
> > struct fwnode_handle *dev_fwnode;
> > bool asd_fwnode_is_ep;
> > bool sd_fwnode_is_ep;
> > + struct device *dev;
> > const char *name;
> >
> > /*
> > @@ -113,7 +114,28 @@ static bool match_fwnode(struct v4l2_async_notifier *notifier,
> >
> > fwnode_handle_put(dev_fwnode);
> >
> > - return dev_fwnode == other_fwnode;
> > + if (dev_fwnode != other_fwnode)
> > + return false;
> > +
> > + /*
> > + * We have an heterogenous match. Retrieve the struct device of the
> > + * side that matched on a device fwnode to print its driver name.
> > + */
> > + if (sd_fwnode_is_ep)
> > + dev = notifier->v4l2_dev ? notifier->v4l2_dev->dev
> > + : notifier->sd->dev;
>
> Have you considered passing the device directly ? seems notifier is
> only used for that...
Yes, but I thought that match functions could use the notifier for other
purposes in the future, so I think this approach is more versatile.
> Apart this small nit, for the series
> Reviewed-by: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx>
>
> > + else
> > + dev = sd->dev;
> > +
> > + if (dev && dev->driver) {
> > + if (sd_fwnode_is_ep)
> > + dev_info(dev, "Driver %s uses device fwnode, incorrect match may occur\n",
> > + dev->driver->name);
> > + dev_info(dev, "Consider updating driver %s to match on endpoints\n",
> > + dev->driver->name);
> > + }
> > +
> > + return true;
> > }
> >
> > static bool match_custom(struct v4l2_async_notifier *notifier,
--
Regards,
Laurent Pinchart
