Re: [PATCH v2 3/4] media: v4l2-async: Log message in case of heterogenous fwnode match

[Jacopo Mondi <jacopo@jmondi.org>]

Hi Laurent,

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@ideasonboard.com>
> ---
>  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...

Apart this small nit, for the series
Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Thanks
   j

> +	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
>